Introduce desired State for runner

[tarfu]

This PR should introduce a desired state functionality to the runner and state.

[tarfu]

Having Heap would make that a lot more convenient. Probably will have another generic parameter on state for what the maximum number of subscribers there are per desired-state/state, which most likely would be the number of different runners there are, and you need to define when initializing. At least, I don't see a way to make it dynamic without heap.

[MathiasKoch]

Yeah, we have a strict heapless approach at Blackbird, so at least for us, we will have to work around that 😔

[tarfu]

Yeah, I have the same premise, as it makes it a lot easier to manage ram and not worry about that parts of something leaking heap in some driver.

If you know for sure you only have one, you need to notify the approach already in with the waker like for link_state works nice but unfortunately not really for the possibility of multiple subscribers.

[MathiasKoch]

I am not sure, I quite understand the issue you are describing?
Could you either elaborate, or give an example?

[tarfu]

What I meant is that the single waker (which is already in the state right now) is nice for the implementation if one party is waiting for a change in the state, but would fight to be the correct waker. Waker registration would overwrite the old waker and wake it, then the old waker would wake and look if it is in the right state and sets itself as the new waker triggering the other waker again and play ping pong. It would work, but would not be really nice for computing resources.

Wasn't in the mood of programming today and worked on a PCB I still have to finish.

[tarfu]

This should see progress again as one of my projects ended and that freed up quite a bit of time.

[MathiasKoch]

No rush from my end, but nice to hear!
I hope to find some time to chime in, once you have established a working foundation for us to work on :)

[tarfu]

It's still a lot uglier than I hoped for, but at least something to discuss on to get progress.

[MathiasKoch]

Wouldn't it be possible for us to just define som MAX_STATE_LISTENERS as a const based on either available services for a module, or just a fixed number? I assume the stack usage overhead of e.g. 3 vs 6 is minimal for a PubSub of unit variant enums (1 byte)

[tarfu]

We could set it as a constant and if it is ever not enough, just increase it would be a lot less generic parameters pain. But if we want then to be able to change it and keep the API, we would then need a feature set for that or something like that.

[MathiasKoch]

I think, at least for now, it makes sense to just pick a "high-enough" number, to reduce the pain of generics at a rather low stack overhead cost.

[MathiasKoch]

Other than those comments, I think the current implementation looks good! With a fixed number for MAX_STATE_LISTENERS I also think looks reasonably simple

[MathiasKoch]

Does it work to set the desired operational state directly to one of the final values, or do you need to do one step at a time as your example?

[tarfu]

I think, at least for now, it makes sense to just pick a "high-enough" number, to reduce the pain of generics at a rather low stack overhead cost.

Would you completely remove the generics, or just set the constant in the asynch/mod.rs and use it in the 2 relevant new()? This would keep them in the state but reduce complexity outside. Or just completely remove it?

Does it work to set the desired operational state directly to one of the final values, or do you need to do one step at a time as your example?

My goal is that you just set what you want, and the runner figures out what the steps are it needs to do. It would be a classic thing for a fall through switch in C 😆 .

[MathiasKoch]

Would you completely remove the generics, or just set the constant in the asynch/mod.rs and use it in the 2 relevant new()? This would keep them in the state but reduce complexity outside. Or just completely remove it?

I think I would completely remove it, and just have a single const MAX_STATE_LISTENERS: usize = 5 that is used where the pubsub is mentioned

My goal is that you just set what you want, and the runner figures out what the steps are it needs to do. It would be a classic thing for a fall through switch in C 😆

Awesome! I would expect it to work that way as well

[tarfu]

Might want to think about a backchannel to the user if something went wrong, so the user can react and knows that for example, a state transition went wrong.

[tarfu]

Currently, is_alive is a bit unstable. Not sure why, maybe because it was powered down and something went wrong then.

[tarfu]

@MathiasKoch have a look as soon as you got time. This is already a bit bigger as it should be and has stuff not really on topic but needed to be done anyways ...

[MathiasKoch]

Awesome 👍
Already started looking a bit, but I will take a thorough look soon :)

[MathiasKoch]

Hmm.. Think I would prefer if this had either a retry counter or a timeout..? This seems like something that can get stuck for infinity?

[tarfu]

It should have a timeout in the transition match. Extracted it to be less code in the match.

[MathiasKoch]

This looks great!
It does have a path dependency on ublox-sockets.. Could we change that to a git dep before merging?

[tarfu]

This looks great!

It does have a path dependency on ublox-sockets.. Could we change that to a git dep before merging?

It is patched in the patch section but sure we can. I think there is also a PR open in ublox-socket for a embassy time bump.

[MathiasKoch]

This looks great!
It does have a path dependency on ublox-sockets.. Could we change that to a git dep before merging?

It is patched in the patch section but sure we can. I think there is also a PR open in ublox-socket for a embassy time bump.

Ahh, sorry i didn't notice that! Patch section should be okay. I just stubled upon the CI failing due to this..

[MathiasKoch]

Nice job on this! Can't wait to see a network / data API 😀