-
Notifications
You must be signed in to change notification settings - Fork 43
Direct async calls proof of concept #511
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
base: main
Are you sure you want to change the base?
Direct async calls proof of concept #511
Conversation
ed58feb to
15e6533
Compare
15e6533 to
5359921
Compare
| fn set_level(&mut self, pin: u8, value: bool) -> impl Future<Output = ()>; | ||
| } | ||
|
|
||
| pub struct Sender<'channel> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The corresponding traits could be made more generic to make these implementations generic over the message type.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I considered that but to what end? The generic service impl would just no-op? The only purpose I could foresee would be vendor-defined messaging, but in that case, should the vendor simply add a side channel to their own types that implements the sender and receiver traits?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The receiver and sender traits in this example are specialized versions of generic EventReceiver<T> and EventSender<T>. This would allow doing blanket implementations like impl<T> EventReceiver<T> for DynPublisher<T> to simplify development. I would also argue that the message type is the real API here. So different services/devices can share EventReceiver<ConcreteType> instead of creating their own incompatible traits. Though I think we'd also want an Event trait, even if it's just a marker trait.
|
|
||
| pub async fn wait_next(&mut self) -> (&'device Mutex<NoopRawMutex, D>, Event) { | ||
| let futures = | ||
| heapless::Vec::<_, MAX_SUPPORTED_DEVICES>::from_iter(self.devices.iter_mut().map(|(r, _)| r.wait_next())); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Futures can end up arbitrarily large so I'm worried that this could blow the stack and we wouldn't know until runtime. Thoughts on possibly introducing a macro/wrapper type that uses size_of to check the total size of the vec at compile time?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this a different risk than when calling any other generic future?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not really, though I think the risk is more acute here because of the multiplication. But I guess that's a broader discussion to have.
asasine
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking good so far!
What do you think of expanding this example to showcase the vendor customization process? I think we can essentially bifurcate into an ODP module and a vendor module. Provide the traits, service impls, and any "default" impls in the ODP mod, provide new types that reuse what's possible but extend behavior in the vendor mod, and a main function that uses the ODP tasks with the vendor types.
| pub struct Sender<'channel> { | ||
| publisher: DynImmediatePublisher<'channel, Event>, | ||
| } | ||
|
|
||
| impl<'channel> Sender<'channel> { | ||
| pub fn new(publisher: DynImmediatePublisher<'channel, Event>) -> Self { | ||
| Self { publisher } | ||
| } | ||
| } | ||
|
|
||
| pub struct Receiver<'channel> { | ||
| subscriber: DynSubscriber<'channel, Event>, | ||
| } | ||
|
|
||
| impl<'channel> Receiver<'channel> { | ||
| pub fn new(subscriber: DynSubscriber<'channel, Event>) -> Self { | ||
| Self { subscriber } | ||
| } | ||
| } | ||
|
|
||
| impl EventReceiver for Receiver<'_> { | ||
| async fn wait_next(&mut self) -> Event { | ||
| loop { | ||
| match self.subscriber.next_message().await { | ||
| WaitResult::Message(msg) => return msg, | ||
| WaitResult::Lagged(n) => { | ||
| warn!("Receiver lagged by {n} messages"); | ||
| } | ||
| } | ||
| } | ||
| } | ||
| } | ||
|
|
||
| impl EventSender for Sender<'_> { | ||
| fn on_plug(&self, power_mw: i32) { | ||
| self.publisher.publish_immediate(Event::Plug(NewContract { power_mw })); | ||
| } | ||
|
|
||
| fn on_unplug(&self) { | ||
| self.publisher.publish_immediate(Event::Unplug); | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think you actually need a new type, just to implement the trait on the inner itself
| pub struct Sender<'channel> { | |
| publisher: DynImmediatePublisher<'channel, Event>, | |
| } | |
| impl<'channel> Sender<'channel> { | |
| pub fn new(publisher: DynImmediatePublisher<'channel, Event>) -> Self { | |
| Self { publisher } | |
| } | |
| } | |
| pub struct Receiver<'channel> { | |
| subscriber: DynSubscriber<'channel, Event>, | |
| } | |
| impl<'channel> Receiver<'channel> { | |
| pub fn new(subscriber: DynSubscriber<'channel, Event>) -> Self { | |
| Self { subscriber } | |
| } | |
| } | |
| impl EventReceiver for Receiver<'_> { | |
| async fn wait_next(&mut self) -> Event { | |
| loop { | |
| match self.subscriber.next_message().await { | |
| WaitResult::Message(msg) => return msg, | |
| WaitResult::Lagged(n) => { | |
| warn!("Receiver lagged by {n} messages"); | |
| } | |
| } | |
| } | |
| } | |
| } | |
| impl EventSender for Sender<'_> { | |
| fn on_plug(&self, power_mw: i32) { | |
| self.publisher.publish_immediate(Event::Plug(NewContract { power_mw })); | |
| } | |
| fn on_unplug(&self) { | |
| self.publisher.publish_immediate(Event::Unplug); | |
| } | |
| } | |
| impl EventReceiver for &DynSubscriber<'_, Event> { | |
| async fn wait_next(&mut self) -> Event { | |
| loop { | |
| match self.next_message().await { | |
| WaitResult::Message(msg) => return msg, | |
| WaitResult::Lagged(n) => { | |
| warn!("Receiver lagged by {n} messages"); | |
| } | |
| } | |
| } | |
| } | |
| } | |
| impl EventSender for DynImmediatePublisher<'_, Event> { | |
| fn on_plug(&self, power_mw: i32) { | |
| self.publish_immediate(Event::Plug(NewContract { power_mw })); | |
| } | |
| fn on_unplug(&self) { | |
| self.publish_immediate(Event::Unplug); | |
| } | |
| } |
| devices: &'storage mut [(R, &'device Mutex<NoopRawMutex, D>)], | ||
| } | ||
|
|
||
| const MAX_SUPPORTED_DEVICES: usize = 4; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems to me that ServiceImplementation should be generic on const N: usize
| fn set_level(&mut self, pin: u8, value: bool) -> impl Future<Output = ()>; | ||
| } | ||
|
|
||
| pub struct Sender<'channel> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I considered that but to what end? The generic service impl would just no-op? The only purpose I could foresee would be vendor-defined messaging, but in that case, should the vendor simply add a side channel to their own types that implements the sender and receiver traits?
|
|
||
| pub async fn wait_next(&mut self) -> (&'device Mutex<NoopRawMutex, D>, Event) { | ||
| let futures = | ||
| heapless::Vec::<_, MAX_SUPPORTED_DEVICES>::from_iter(self.devices.iter_mut().map(|(r, _)| r.wait_next())); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this a different risk than when calling any other generic future?
Proof of concept for calling async trait functions directly. This example introduces two device traits (
power::Deviceandio_expander::Device) - which are basic traits for a power policy device and an IO expander device, two simple service implementations that use these traits, and a mock device that implements both device traits. Both service implementations directly call async trait functions without using device IDs or messaging as intermediates.Notes
StaticCelland everything is declared with normallet _ = ...statements. This is only possible if everything is in the same embassy task. Using multiple embassy tasks will require'static.'static.'staticto work. Additionally this requires the use ofdynto break a recursive type that arises between the channel and the device.EventSendertrait. This is to ensure that device implementations don't come up with their own bespokeEventReceivertypes which would all be incompatible with each other.