Feedback from my recent usage of Ergot

[hydra]

As per the matrix chat, I'm creating a discussion topic with my feedback that came about as a result of recent of attempting to use ergot and adding embassy-net-udp and tokio-udp support (PRs: #150 / #151 / #152 )

These are just my notes I made while trying to understand and work on Ergot for the first time.

They are not meant as a criticism, just a collection of thoughts, right or wrong.

The goal here is either constructive discussion, and maybe some actionable tasks to improve Ergot for everyone.

the 'major concepts' talk about the stack, sockets, address, frames, profiles, interfaces, etc, great. however the examples, in have files with and suffixes. it would probably be good in the docs to define what a 'client' is, and what a 'router' is and what their responsibilities are. currently I'm trying to work this out by reading the code...

the naming of the examples and what they currently do doesn't quite make sense.

one thing that would really help, would be some module-level documentation at the top of each example, giving an overview of what it's supposed to do and what the hardware requirements are, if any.
additionally, document any required environment variables.
some examples produce no output unless you run them with RUST_LOG=info or similar. (ergot-client, ergot-router, etc).
ergot-seed-router - I've no idea what that would do, and opening it up and quickly scanning it doesn't help that much either.
I have to compare the main.rs of ergot-router and ergo-seed-router to find out.

the word 'seed' isn't mentioned in the section of the book.

perhaps the word 'node' should be used more? this is a common term in the networking world.
'edge node', 'router node', 'hub node' etc?

it's a bit tricky to see the trace/log messages when there's a mix of both defmt::info and log::info in the codebase.

documentation on embassy_usb.rs\eusb_v0_5::tx_worker are incomplete, finished mid-sentence.

/// Takes a bbqueue from the NetStack of packets to send. While sending,
/// we will timeout if

I have a feeling the API for (3rd parameter) shouldn't just return a bool, but should return some kind of error, since it manifests as 'NoRoute' when there is a route, but the buffers are full.

fn broadcast<SendSockets, SendProfile>(
    sockets: &mut List<SocketHeader>,
    hdr: &Header,
    mut sskt: SendSockets,
    smgr: SendProfile,
) -> Result<(), NetStackSendError>
where
    SendSockets: FnMut(NonNull<SocketHeader>) -> bool,
    SendProfile: FnOnce() -> bool, 

I feel that would be useful, in combination with some and logging when errors occur, it took a little while to even find the cause of the which at the time it happened was misleading, since other packets were getting though to the same host.

the abbreviations ty, bor (in the send_ty and send_bor methods, in NetStack were non-obvious, especially bor. it turns out they mean types and borrowed respectively. I feel it might be better to rename the methods. these days with auto-completing IDEs I think it's better not to use opaque/cryptic names and just use the full words. same goes for ctr and res for variable names, I personally prefer counter and result, this is especially true in the demos/examples which are source files that are designed to be as clear as possible and read often.
IMHO it's more important to have well named methods and structures than well named local variables, since local variables don't appear in things like docs, call-stacks and ide auto-complete prompts. also humans and LLMs may be able to understand the API better too.

another thought occured today, some documentation/overview around what all the various buffers are for and how/why you can specify sizes for them would be really helpful, as it can be hard to remember what controls what.

e.g. OUT_QUEUE_SIZE, MAX_PACKET_SIZE, RECV_BUF, SCRATCH_BUF, and the N args on the bounded_receiver.

for example, when dealing with an 'InterfaceFull' error, when sending a broadcast message, I have a hard time looking up which buffer to adjust.

need an example of a router that can route between two or more different interfaces, e.g. TCP + USB, or USB + 2 serial, or TCP + UDP.
this is the use-case of a server app that talks to a GUI client (via TCP/UDP) and the server app also talks to one or more embedded control boards (via n amount of UDP sockets, USB or serial ports)

I have a feeling we need a better/additional error kind than InterfaceFull which should be used when the MTU is exceeded.
My thinking is that MTUExceeded is an error that cannot be retried with the same data, whereas seems like it might not be full in the future and a retry is applicable.
currently the profile implementations reply with InterfaceFull when InterfaceSink::send_* methods return any error, see the implementations in cobs_stream and framed_stream.
I think when the grant fails, interfacefull is applicable. but when serialization fails then it's MTU related.

the book does not mention 'discovery'

there are no demos that send unicast messages.

it's not clear how to handle discovery or how to handle a client that is restarted, since port allocation can occur in a different order when a client is restarted and a server could be sending unicast messages to the previously assigned port number resulting in errors on the client without the server knowing about it. if a client is restarted between the server's device discovery attempts the server still won't know it needs to rediscover sockets.
do all servers need to periodically do socket discovery and then handle differences? feels messy.
perhaps clients should be able to request a specific port number?

every demo should have module documentation on what it's attempting to demonstrate, potentially also how to run it and what else it needs to function. many demos require 2 or more programs to run, for example: ergot-bridge-client-tcp needs another demo to run, but it's up to the user to trawl though the other demos to find something it will work with.