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

Not waiting on a notifier hangs stepTiming in simulation #7660

Open
virtuald opened this issue Jan 8, 2025 · 4 comments
Open

Not waiting on a notifier hangs stepTiming in simulation #7660

virtuald opened this issue Jan 8, 2025 · 4 comments
Labels
type: bug Something isn't working.

Comments

@virtuald
Copy link
Member

virtuald commented Jan 8, 2025

This hangs:

from wpilib.simulation import stepTiming
import hal
hal.initializeNotifier()
stepTiming(0.2)

@ThadHouse thinks it shouldn't hang the program. @calcmogul in #3874 (comment) seems to think it should hang it. Either it should be fixed or documented.

@virtuald virtuald added the type: bug Something isn't working. label Jan 8, 2025
@ThadHouse
Copy link
Member

High level robot code has startSingle. Which means a stopped notifier is 100% perfectly valid user code. There should be no requirement that all notifiers are running to not hang sim.

@ThadHouse
Copy link
Member

Actually, I take my above comment back. High level notifiers still always call WaitForNotifierAlarm, because they always have their own thread.

So this is purely a HAL level issue.

@virtuald
Copy link
Member Author

virtuald commented Jan 8, 2025

I think that the implicit contract makes sense because missing a notifier event is undesirable. If the code leveraging the notifier performs a long-running task, the time should not increment until that task eventually calls HAL_WaitForNotifierAlarm, ensuring synchronization. Incrementing time before calling HAL_WaitForNotifierAlarm would result in time becoming out of sync.

Additionally, this implies that it would be a bug to perform a completely blocking operation without ultimately waiting on the notifier.

I think we should document the behavior more throughly and move on.

@virtuald
Copy link
Member Author

virtuald commented Jan 8, 2025

If notifiersWaiterCond.wait_for(ulock, std::chrono::duration<double>(1)); in hal::WaitNotifiers times out and doesn't exit the loop, I think it should print a warning telling the user that something is using a notifier incorrectly (and add this discussion there, or a pointer to it) .

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Something isn't working.
Projects
None yet
Development

No branches or pull requests

2 participants