-
Notifications
You must be signed in to change notification settings - Fork 10
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feature(async): ppp support using embassy-net-ppp & embassy-at-cmux #101
Conversation
Hey it looks nice will build an example for my setup as well. Maybe feature gating both the data_established and the internal network stack as well as the PPP stuff, so you can decide what you want and reduce the size to a minimum? |
OK, after looking a bit closer, it is nice that it runs but really needs some moving of parts cleaning and putting all the wiring and implementation into a module. For skipping the at_init we could feature gate it for non PPP, but I'm not sure if it really is a dealbreaker to have it. A lot of the stuff should not harm the PPP session and are set in your program as well. But really cool POC! |
Absolutely, this is very much just a PoC! So far my finding are, as you mentioned:
I think an either or approach to |
… batteries included new functions that sets up ATAT and all related resources
Take a look at my latest commit changes. I think I have managed to clean it up quite a lot for both the PPP mode and the regular ublox mode. I have made changes such that the Overall, I am very happy with the result of this so far. Stuff on my to-do:
|
It looks a lot nicer! How do you feel about the PRs it is depending on, would you think they are ready to try to be merged with a bit of work like renaming and changing descriptions? For the features, I would go with two features:
The state machine definitely has room for improvement, but just feature gating the stuff not needed should work without issue. What is your baud rate an issue? It is always set up in the HAL layer, and I don't think there is a way to change it for a driver. |
ed9e756
to
67adb83
Compare
Nice additions and refactorings! |
8c9710e
to
cc6ceca
Compare
cc6ceca
to
8206f13
Compare
…port to control handle
@tarfu FYI
This is still very early WIP, but perhaps you have some ideas you want to chip in.
Concerns uncovered so far from my end, that I would like to see addressed by us at some point:
The
OperationState::DataEstablished
is not used in PPP mode at all, as we do our own dial-up usingATD*99#1
I think it might make sense to feature-gate the last state behind flags that actually makes use of the internal TCP/IP stack?
ublox-sockets
,ublox-mqtt
etc. family of feature flags?The initialization/resource creation is a bit cumbersome/bloated
Currently, the creation of all the resources during program initialization feels a bit verbose and brittle.
I think we should be able to wrap this much nicer together somehow.
Eventually, I would love to see a situation where we would just get back a set of (
Device
,Runner
,Control
), where:Runner
is owning and propagating the full combination of cmux, ppp and the existing ublox runner through an internal select/join (maybe even the uart ingeress?)Control
is the same struct as when using the internal TCP/IP stack (network state, operators etc.)Device
is the struct you would pass toembassy-net::Stack
It makes use of FactbirdHQ/atat#203