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

Add Signal to rtic-sync #934

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

Add Signal to rtic-sync #934

wants to merge 7 commits into from

Conversation

AdinAck
Copy link

@AdinAck AdinAck commented May 7, 2024

Add the Signal structure to rtic-sync.

A Signal has unlimited writers and one reader, many writes can occur before a read, all overwriting the previous.

A reader can asynchronously wait for a write, requiring the write to occur during the lifetime of the wait future, or immediately resolve from previous writes (up to the user).

That's it, thanks!

Copy link
Collaborator

@korken89 korken89 left a comment

Choose a reason for hiding this comment

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

Thanks for adding! I have some comments that need looking into.

rtic-sync/src/signal.rs Outdated Show resolved Hide resolved
rtic-sync/src/signal.rs Outdated Show resolved Hide resolved
rtic-sync/src/signal.rs Outdated Show resolved Hide resolved
Copy link
Collaborator

@korken89 korken89 left a comment

Choose a reason for hiding this comment

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

Thanks for the updates! Can you have a look at adding tests?
You can copy the tests from channel if you need inspiration.

@AdinAck
Copy link
Author

AdinAck commented May 8, 2024

Sure! I also planned on adding an entry to examples later, I can include that in this PR along with the tests if you like.

@AdinAck
Copy link
Author

AdinAck commented May 9, 2024

Ok I think I added all the necessary tests.

I'm not quite sure about the soundness of the last test though, it works on my machine and the GitHub action, basically I yield the test thread to get tokio to poll the reader future waiting for a "fresh" write before the write occurs. What are your thoughts?

#[tokio::test]
async fn waiting() {
    static READER: StaticCell<SignalReader<u32>> = StaticCell::new();
    let (mut writer, reader) = make_signal!(u32);

    writer.write(0xaa);

    let reader = READER.init(reader);
    let handle = tokio::spawn(reader.wait_fresh());

    tokio::task::yield_now().await; // encourage tokio executor to poll reader future
    assert!(!handle.is_finished()); // verify reader future did not resolve after poll

    writer.write(0xab);

    assert!(handle.await.is_ok_and(|value| value == 0xab));
}

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.

None yet

2 participants