v0.1 and onward Interface discussion

[calebbourg]

Here is a brainstormed first pass at what a public API might look like for the 0.1 version (and onward) of the crate. It will continue to evolve and can radically change until we reach our first stable 1.0 release.

All input and suggestions are welcomed and encouraged.

Copied over from this Google doc

Some notes on API best practice guidelines:

• When you using compound names for functions (Ex. firmware_version) prefer 3 words or less where possible.

Proposed Public Functions/Methods

These are some ideas on what our public API could look like. I’ve added some thoughts on underlying implementations where I had some questions but for the most part attempted to keep things at a high level. Likely some of our other Proposed Rust Crate Structs not mentioned here will turn up behind the scenes.

The intent here is to satisfy the following v0.1 requirements:

• Crate is available on Crates.io
• Consumer of crate can join/leave a provided WIFI network (non-enterprise)
• Consumer of crate can monitor WIFI network connection status
• Consumer of crate can send a valid TCP data stream to remote server
• Crate can support both IP address and host name

Structs

Wifi - handles interaction with a Wifi network.

Responsibilities:

• Joining network
• Leaving network
• SSID/Passwords
• DNS - This seems like a reasonable place for this, though it implies an underlying UDP implementation. One option could be that we remove DNS resolution from v0.1 and come back to it in a later release (probably v0.2) when we have a better sense for how everything works together. If we decide to keep it in scope then I’m open to other ideas.

Methods and Parameters:

• init - returns a Wifi struct instance which is configured according to the given arguments. Joins network.
  • params:
    • Some sort of data bus handler Ex. SPI (enabled)
    • A Configuration struct. Could include optional DNS IP
  • Compile Time Error Scenarios:
    • misconfiguration
  • Runtime Error Scenarios:
    • Bad SSID
    • Bad Passphrase
    • Timeout
    • Does not meet minimum NINA firmware version requirements
• join - joins a WIFI network
  • params:
    • ssid
    • passphrase
  • Compile Time Error Scenarios:
    • SSID 32 character length and valid ASCII
    • Passphrase 8-63 character length and valid ASCII
  • Runtime Error Scenarios:
    • TBD invalid Character provided for SSID
    • Could not communicate with the target NINA device on the selected data bus
    • WIFI network not found
    • Failed to join WIFI network (invalid passphrase, timeout, etc.)
• leave - leaves currently joined WiFi network. Transitions Wifi struct to disabled. Returns ().
  • Compile Time Error Scenarios:
    • N/A
  • Runtime Error Scenarios:
    • N/A
• resolve - resolves hostname to IP
  • params:
    • hostname
    • optionally takes a configuration struct with DNS server IP
  • Compile Time Error Scenarios:
    • potentially need to validate that the hostname is not an empty string.
    • misconfiguration
  • Runtime Error Scenarios:
    • host not found
    • network failure
    • host lookup timeout
• firmware_version - returns current NINA firmware version.
  • Compile Time Error Scenarios:
    • N/A
  • Runtime Error Scenarios:
    • Could not communicate with the target NINA device on the selected data bus

Client (or TcpClient)

Responsibilities:

• Connecting to TCP server
• Disconnecting from TCP server
• Sending TCP data stream to connected server
• Polling connection state

Methods and Parameters:

• init - returns instance of struct. V0.1 Assumption: We will only handle one client connection at a time.
  • params:
    • a Wifi instance - should indicate somehow that a network has been joined
    • IP Address
    • Port
  • Compile Time Error Scenarios:
    • misconfiguration
• connect - connect to a TCP server with IP/Port
  • Compile Time Error Scenarios:
    • N/A
  • Runtime Error Scenarios:
    • Wifi network disconnected
    • Wifi network unavailable
    • TCP server connection failed
    • Network timeout (think through layering of errors and exposure to crate user network vs data bus timeout error)
• connect_with_hostname - connect to a TCP server with a hostname. Will use Wifi.resolve internally to resolve hostname
  • params:
  • hostname
  • Compile Time Error Scenarios:
    • N/A
  • Runtime Error Scenarios:
    • Wifi network disconnected
    • Wifi network unavailable
    • TCP server connection failed
    • Network timeout
    • Could not resolve hostname
• disconnect - terminate current connection with TCP server
  • Compile Time Error Scenarios:
    • N/A
  • Runtime Error Scenarios:
    • Network timeout
• send_data - send data to a connected TCP server
  • params:
    • data
  • Compile Time Error Scenarios:
    • Buffer size limit exceeded
  • Runtime Error Scenarios:
    • Wifi network disconnected
    • Wifi network unavailable
    • TCP server disconnected
    • Network Timeout
• receive_data - receive data from a connected TCP server
  • return param:
    • data
  • Compile Time Error Scenarios:
    • Buffer size limit exceeded
  • Runtime Error Scenarios:
    • Wifi network disconnected
    • Wifi network unavailable
    • TCP server disconnected
    • Network Timeout
• connected - determines whether the client is connected or not
  • Compile Time Error Scenarios:
    • N/A
  • Runtime Error Scenarios:
    • Wifi network disconnected
    • Wifi network unavailable
    • TCP server disconnected

Proposed Private Functions/Methods

Bus possibly NINABus

Responsibilities:

• Handles protocol byte communication between ESP32-WROOM-32U/UE and RP2040 for e.g. SPI

Methods and Parameters:

• call_function(funct, parameters)

[mccormick-wooden]

Couple quick thoughts:

• I assume for TCPClient, the idea would be to wrap something like TcpStream instead of rolling something completely custom?
• I noticed that TCPClient has no read/receive, is that not needed?

[calebbourg]

Thank you for taking a look and for the feedback @mccormick-wooden!

The intent behind TcpClient is to provide an interface for interacting with the device firmware TCP client API.
So a method like TcpClient.connect might, via SPI bus communication, invoke the nina-fw startClientTcp function or some combination of functions to start the firmware TCP client.

As our requirements currently stand we did not include the notion of receiving data in version 0.1.
I added our goals to the description as well.

One of our larger design goals for the crate is to provide meaningful and usable functionality in each release and so I think there could be an argument for providing the ability to receive data. Our thoughts were that sending data (without receiving) still provides usefulness while keeping the v0.1 scope small. Open to thoughts on that.

[mccormick-wooden]

to provide an interface for interacting with the device firmware TCP client API.

Got it! I think I misunderstood the intent, thanks for clarifying.

Our thoughts were that sending data (without receiving) still provides usefulness while keeping the v0.1 scope small. Open to thoughts on that.

I don't have a strong opinion (as just a casual observer for now) and am a little out of my depth wrt embedded. That said, TCP is inherently a bi-directional protocol and I'm curious about what issues might happen if it's participating in a TCP connection but not handling the read buffer (even in an extremely basic way). In Linux when the read buffer is full the kernel just doesn't ack additional packets but I'm not sure about FreeRTOS (which I think is the kernel for your target here). It's probably the same or similar behavior, but not sure.

[jhodapp]

What's cool about the design here is our crate isn't implementing any lower level notion of TCP or sockets, it's simply a handler layer that interfaces an RP2040 target board with an ESP32-WROOM-32U/UE series of coprocessor boards over the SPI bus. So even if we don't quite handle TCP reads with version 0.1, TCP reads will still occur on the other board - it'll just sit there in the TCP socket's buffer. And then a 0.2 release would pretty quickly bring the ability for a user of the crate to perform a TCP read.

You might be interested in checking out our proof-of-concept implementation that we used to get familiar with working with the coprocessor boards - it can actually both send and receive TCP data today.

[jhodapp]

This is a great initial proposal @calebbourg, thank you for this! Here are some of my initial thoughts in continuing to brainstorm our 0.1 API:

Wifi

Would we ever want more than one instance of Wifi to exist in a single embedded application using our crate? If not, we probably want a single static instance of it and no use need for new.

Client

For the Client struct we could make this be a base struct with traits that future structs could implement. For example, we could have TCPClient be a base that we then implement different types of TCP clients on top of, e.g. HTTPClient. This will allow our library to provide even more useful functionality for future releases and intentionally sets up our API to do so. What we need to think through is if it makes sense to have TCP and UDP clients be related through a common base, or we have two distinct bases TCPClient and UDPClient.

connect -> Do we want this to block until a connection is successful or return immediately? What kind of return type do we want? I could see us creating a ConnectionStatus or ConnectionState struct that could reflect a TCPClient's current connection state.

get_connection_state -> Same here - do we want it to return a ConnectionStatus or ConnectionState struct instance?

Errors

For all API functions we also need to define the return and error types that we'll need for v0.1.

Again, I want to reiterate how strong a start this is and I look forward to iterating towards a strong first interface definition together!

[jhodapp]

It seems it's built on top of UDP and a new connection-based transport protocol called QUIC. So perhaps our model still works but HTTP/3 would just be one of a UDPClient in our hierarchy.

More info from the HTTP/3 Wikipedia page.

[calebbourg]

This is really great. Thank you.

• For Wifi I agree and that's an excellent point. We will still need to instantiate the single instance and I like the idea of providing a method. Doing so allows us to have more control over which parts of the internals are private. Maybe something like init ? It would serve a similar purpose though perhaps the name expresses something more inline with our intent than new. init also shows up in other crates I've seen. We can look into setting up Wifi as a singleton as well.

• connect/get_connection_state The way I originally envisioned these were that connect would return immediately and then consuming code would use get_connection_state to poll until the firmware client is started and successfully connected to a server. One advantage to this would be that it would allow others to take action on any iteration of the polling loop. Not super convinced that it's the best way. Curious what you think and how this sort of thing is typically done. In either case I like the idea of returning some sort of struct ConnectionStatus or ConnectionState from both.

• Completely agreed on providing errors. Maybe we can brainstorm a list of the types of failure the v0.1 functionality could experience and then codify a good set here in this discussion?

[jhodapp]

• For Wifi I agree and that's an excellent point. We will still need to instantiate the single instance and I like the idea of providing a method. Doing so allows us to have more control over which parts of the internals are private. Maybe something like init ? It would serve a similar purpose though perhaps the name expresses something more inline with our intent than new. init also shows up in other crates I've seen. We can look into setting up Wifi as a singleton as well.

I think an init function would work very well here. I think a good model for Wifi is the Spi struct from rp-hal. They include an init function and I also like how they model doing initial config. Obviously it's not an exact equivalent, but I think there's many great things that do directly apply here.

One other thing we can learn from but I don't think we should model as closely (I'm open to changing my opinion on this) is the WiFiNINA API from Arduino. They have a begin that seems equivalent to an init and a paired end function that simply turns WiFi off. There are certainly pros to this API in that it's widely familiar and reused even in competing products (e.g. Particle). But to me, that's not necessarily a good enough reason to continue to copy it for our Rust-based API.

• connect/get_connection_state The way I originally envisioned these were that connect would return immediately and then consuming code would use get_connection_state to poll until the firmware client is started and successfully connected to a server. One advantage to this would be that it would allow others to take action on any iteration of the polling loop. Not super convinced that it's the best way. Curious what you think and how this sort of thing is typically done. In either case I like the idea of returning some sort of struct ConnectionStatus or ConnectionState from both.
• Completely agreed on providing errors. Maybe we can brainstorm a list of the types of failure the v0.1 functionality could experience and then codify a good set here in this discussion?

I think this sounds good and it doesn't prevent us from also trying to have an event-based (i.e. interrupt-driven) API as well if that's tenable for our hardware and nina-fw.

One thing to consider is just shortening get_connection_state to state since it'll be associated with the Wifi struct and do the same thing for get_connection_status to be just status. This is what WiFiNINA's API does and I think it provides for a clear API UX around these two functions. In fact, they don't even provide a state function, only status. Instead, they provide a Client.connected method which I like quite a bit. Thoughts on this?

[calebbourg]

I think I like using init for now because it matches what I see in most of the Rust crates I've looked at.
I have been referencing the Arduino API a bunch when thinking through this and it has been invaluable.

In terms of checking the status/state of the client I think providing a Client.connected method sounds great. For this release I think that will satisfy what we need. In the future, if we determine that we need a more expressive API for exposing different states, we can modify this. I changed get_connection_state to just connected

[jhodapp]

Sounds great. One of the cool things about Rust return types is it can be an Option that has a bool on Ok and some expressive Err type on error. So in that regard, we can keep the API really simple with a function like connected but also express far more than just true/false.

[calebbourg]

In regards to how we want to abstract the transport concept, I agree with @mccormick-wooden that it may be something that reveals itself over time. It's definitely valuable to start thinking about it now and to continue to do so. Depending on our testing strategy it may make sense to provide traits for this early on in order to be able to better stub and mock parts of the system.

[jhodapp]

I think if we were implementing the protocol stack on the ESP32 firmware side this would absolutely make sense. But since we're not, I'm not sure that there's anything for us to abstract or design for. Am I missing something that you're seeing?

[calebbourg]

In terms of Error strategy, I'd like to propose starting with the following steps:

1. Determine a basic list the types of failures we can expect
2. Update relevant methods from the above to return Result structs which will return appropriate Errors from our list. This will allow us to take advantage of what Rust provides for Result and related structs and will facilitate error handling as well as functional composition. It could be that we define our own type of Result but for now it can we can just think of it as "something with an interface that matches Result"

[jhodapp]

I love this plan Caleb. This sounds like something we could work on together tomorrow night during the group session if that sounds good to you.

[calebbourg]

Sounds great to me

[calebbourg]

@jhodapp

I've been thinking a bit after our conversation today.

What do you think about adding a CommandHandler interface (or even NinaCommandHandler) which defines functions that map closer or even 1:1 to the commands provided by the NINA firmware. We would only need to support the subset of commands we need for any given feature.

We could then define a lower level SPI interface that handles the actual SPI bus communication and our CommandHandler would be an abstraction on top of that.

It would provide a way to decouple invoking NINA commands from the underlying data bus protocol since they are separate concerns. So in theory CommandHandler can delegate to SPI or whatever other underlying protocols we decide we want to support in the future.

Top level interfaces like Wifi would accomplish their goals via appropriate composition of CommandHandler functions and would not need to know anything about the underlying protocols.

So the layers of abstraction would be something like:

• Using the SPI bus: Wifi/Client -> CommandHandler -> SPI(ours) -> SPI (rp-hal)
• Future I2C implementation: Wifi/Client -> CommandHandler -> I2C(ours) -> I2C(rp-hal)

[jhodapp]

Really good thoughts, I think it makes a lot of sense Caleb. Basically mirror the interface on the nina-fw implementation side. It's what we'd end up doing if we implemented both ends of the SPI bus from scratch anyway.

And then our own SPI layer would really be something like a NinaBus interface since it is slightly more purpose-built than rp-hal's generic SPI/I2C bus interfaces.

Am I thinking in line with what you're thinking?