Use mut ref device ref to save stack space

[lulf]

• Remove type annotations no longer needed on states
• Session state no longer depends on Device type
• Shared state is stored in Device rather than on stack instead of as
  part of session state

The advantages with this change is:

• Device no longer need to be moved when processing events, while session state is replaced correctly. I find this more ergonomic, and it still allows user to move the type if desirable
• Stack usage on a simple join example is reduced from 7.34kB to 4.69kB, static usage remains the same
• Flash usage reduced by 3kB
• This is a prereq for async support, where the size of the futures will include all owned state like Shared, which is avoided with this change.

[lthiery]

Wow those are some serious memory improvements - well done!

I need to spend some time getting a feel for how this affects the API but had time for a quick glance right now. The struct Device { .. state: Option<State> } concerns me a bit at first glance.

[lulf]

Wow those are some serious memory improvements - well done!

I need to spend some time getting a feel for how this affects the API but had time for a quick glance right now. The struct Device { .. state: Option<State> } concerns me a bit at first glance.

Yeah, I was not able to avoid it. The main reason is to avoid making State derive Copy/Clone which could have even worse impact. I think using Option this way is necessary because the session and the states takes self instead of &mut self. I could change that as well, but I think there is merit to the current "consume self return different type" style to avoid accidental bugs in the implementation. For the Device itself though, I think not consuming self is fine: only a single point modifies the internal state.

The handle_event fn's now bear the parameterization and also is passed a &mut Shared within the state handling, and this is the primary reason for reduced stack usage.

I found that in my use, the code becomes simpler:

[ivajloip]

Wow, this is some really important improvement, indeed! Thank you a lot for this work! It looks good to me at first read. @lthiery , if you have any concerns, please let us know, otherwise I intend to check it again and merge it in a few days.

[lthiery]

I think using Option this way is necessary because the session and the states takes self instead of &mut self.

This makes a lot of sense. I still wonder if there's a way for state to not take self instead of &mut self but I remember struggling with it quite a bit and not finding a better solution. It feels bad that that shortcoming now forces us to reach for Option, but I think that these improvements are significant enough that we can deal with it. Maybe sometime in the future, the self/&mut self issue can be resolved.

Of course, maybe it will not be resolved and we transition to Ulf's async efforts instead. As he mentions, the compiler builds the state machine for you so much of the current state machine design becomes obsolete in that case.

However, I have not been paying enough attention to Rusts's embedded async development to understand whether there will ever be reasonable reason to hold out against it (and thus whether we should envision maintaining a non-async version of the device stack alongside an async one). My gut tells me that for very resource constrained folks, a very lean embedded runtime will exist, because Rust, but time will tell on that.

[lulf]

I think using Option this way is necessary because the session and the states takes self instead of &mut self.

...

However, I have not been paying enough attention to Rusts's embedded async development to understand whether there will ever be reasonable reason to hold out against it (and thus whether we should envision maintaining a non-async version of the device stack alongside an async one). My gut tells me that for very resource constrained folks, a very lean embedded runtime will exist, because Rust, but time will tell on that.

We're using embassy, which is developed by collaborators across the Rust embedded community.

However, I don't see the async-await implementation replacing a non-async version, primarily because of the requirement of an async runtime, and there will be developers who prefer not using async-await, or simply have stricter requirements.

So my take would be that lorwawan-device can offer both options, and we can try to share as much code between these implementations as possible.

[lthiery]

However, I don't see the async-await implementation replacing a non-async version, primarily because of the requirement of an async runtime, and there will be developers who prefer not using async-await, or simply have stricter requirements.

That sounds reasonable. It will be an interesting exercise then to maintain both and maximize code sharing. I wonder if there's a clever way to invert an async driven design into an sync one.

[lulf]

However, I don't see the async-await implementation replacing a non-async version, primarily because of the requirement of an async runtime, and there will be developers who prefer not using async-await, or simply have stricter requirements.

That sounds reasonable. It will be an interesting exercise then to maintain both and maximize code sharing. I wonder if there's a clever way to invert an async driven design into an sync one.

You can write an executor that just blocks/continously polls a future until completion  (I.e block_on macro from futures crate), but that would be inefficient and not work if you need some other task to run in order to signal the future that data is ready for instance, because the executor can only run one future to completion.

Another approach would be to write Future implementations manually, but that is a really hard programming model which I wouldn't really recommend.

I tried to see if I could use macros for this, but even that is really hard.

Agreed it's annoying to duplicate code, but I think the alternatives are worse right now.

[lthiery]

Agreed it's annoying to duplicate code, but I think the alternatives are worse right now.

Yeah, I can see that... trying to be too clever might not work out and may become unwieldy. I think the simple next step is just to try do incremental refactoring to promote more code-sharing between the state-machine and async-await implementations.