Skip to content

zephyr:embassy: Use a semaphore for the executor #79

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

Merged
merged 1 commit into from
Apr 8, 2025

Conversation

aolowin
Copy link
Contributor

@aolowin aolowin commented Apr 3, 2025

Fixes a potential race condition in the suspend/resume sequence.

@d3zd3z
Copy link
Collaborator

d3zd3z commented Apr 4, 2025

@aolowin I've approve the workflow to run, so you should see the rustfmt fixes that will be needed.

Also, you will need a Signed-off-by line in the commit, which you should be able to add with git commit --amend -s.

The other minor nit is that there should be a space after the colon in the commit title, e.g. zephyr: embassy: ....

@aolowin
Copy link
Contributor Author

aolowin commented Apr 4, 2025

I'm not sure why the build is failing on Semaphore::new(0, 1).unwrap()

The unwrap() is needed on my build to compile and it's used in a few of the samples.

@mrodgers-witekio
Copy link

I'm not sure why the build is failing on Semaphore::new(0, 1).unwrap()

I think this commit removed the need for unwrap: 3a0016d

@aolowin
Copy link
Contributor Author

aolowin commented Apr 7, 2025

I'm not sure why the build is failing on Semaphore::new(0, 1).unwrap()

I think this commit removed the need for unwrap: 3a0016d

Indeed. My build directory was a few commits behind. Thanks!

Fixes a potential race condition in the suspend/resume sequence.

Signed-off-by: Aaron Olowin <[email protected]>
Copy link
Collaborator

@d3zd3z d3zd3z left a comment

Choose a reason for hiding this comment

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

My testing shows this to work well. I like the solution, and this will make it easier to add support for async wait on other semaphores.

Copy link
Member

@cfriedt cfriedt left a comment

Choose a reason for hiding this comment

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

LGTM

@d3zd3z
Copy link
Collaborator

d3zd3z commented Apr 8, 2025

I'm not sure why the build is failing on Semaphore::new(0, 1).unwrap()

The unwrap() is needed on my build to compile and it's used in a few of the samples.

This was unfortunately needed to allow semaphores to be declared as statics, which needs the construct to be const. Result is technically returnable from const, but it is almost impossible to use (at least with stable rust). So, I changed the behavior to panic if the initializer is nonsensical, and just return the Semaphore.

@d3zd3z d3zd3z merged commit b7dacda into zephyrproject-rtos:main Apr 8, 2025
3 of 4 checks passed
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.

4 participants