Skip to content
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

actor API #537

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
Open

actor API #537

wants to merge 9 commits into from

Conversation

japaric
Copy link
Collaborator

@japaric japaric commented Sep 27, 2021

this PR contains a proof-of-concept implementation of the actor API proposed in rtic-rs/rfcs#52
the files added to the examples directory are tests of the feature semantics
a full example, with multiple crates (firmware + testable no_std library), has been included in the actor-example directory

depends on rtic-rs/rtic-syntax#75

@japaric japaric marked this pull request as draft September 27, 2021 13:37
@japaric japaric mentioned this pull request Sep 27, 2021
@japaric
Copy link
Collaborator Author

japaric commented Jan 28, 2022

@rtic-rs/devs this PR has been rebased onto rtic 1.0 and the deps of the firmware have also been bumped 🚀

@japaric japaric marked this pull request as ready for review April 1, 2022 14:17
@japaric japaric changed the title [PoC] actor API actor API Apr 1, 2022
@japaric
Copy link
Collaborator Author

japaric commented May 3, 2022

ping @korken89 this and the rtic-syntax PRs have been rebased

@korken89
Copy link
Collaborator

Now with v1.1 out the door I think it's time to give this the attention for a v1.2 release.

@jscatena88
Copy link

Looking at the actor-example multi-crate example I am curious as to why FakeTempratureSensor is defined as an actor in the actor crate then not listed in the Actors struct. Instead an instance of it saved in Local. Why would you not just save a copy of the Poster in Local and have the periodic task use that to post a message into the Actor system. Seems confusing to have an Actor that isn't quite used as an Actor.

@japaric
Copy link
Collaborator Author

japaric commented Jun 7, 2022

indeed, having the temp sensor as an actor instance makes more sense. updated the example.

japaric and others added 9 commits July 7, 2022 14:27
see rtic-rs/rfcs#52 for details
includes: core proposal and `#[init]` and `memory-watermark` extensions

Co-authored-by: Jonas Schievink <[email protected]>
this way it won't be compiled by `cargo b --examples`
it'll be compiled by 'cargo b --examples --features actor-watermark`
@japaric
Copy link
Collaborator Author

japaric commented Jul 7, 2022

@korken89 review ping. (someone else also asked about when this would be merged in the matrix chat room)

{
fn receive(&mut self, temperature: TemperatureReadingCelsius) {
if temperature.0 >= self.threshold {
self.outbox.post(TemperatureAlert).ok().expect("OOM");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is ok() called here when you could call expect directly on the return from post() i.e.
self.outbox.post(TemperatureAlert).expect("OOM");

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

doing expect on the Result<T, E> value requires that E implements the fmt::Debug trait. doing ok().expect() places no trait requirements on the type E. TemperatureAlert does not implement the fmt::Debug trait so post(TemperatureAlert).expect() would not compile.

deriving Debug on the type and leaving out the ok() is also an option (no pun intended). I tend to write ok().expect() mostly out of bad habit. (less Result::unwrap calls produces smaller (code size) binaries but that's micro-optimizing and this is an example so that hardly matters; as I said: bad habit)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ahh, ok makes sense. Thanks for the reply, I was mainly just curious more than critiquing

@bb010g
Copy link

bb010g commented Jun 2, 2023

What's the status of this, now that v2.0 is stable?

@AfoHT
Copy link
Contributor

AfoHT commented Jun 7, 2023

What's the status of this, now that v2.0 is stable?

The proposed path forward is to create a separate branch for actor, just like other older releases.
I did rebase this actor branch onto latest RTIC v1, then they could be released as alphas or similar.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants