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

Yield in each loop of observe_value #648

Merged
merged 1 commit into from
Nov 14, 2024
Merged

Yield in each loop of observe_value #648

merged 1 commit into from
Nov 14, 2024

Conversation

coretl
Copy link
Collaborator

@coretl coretl commented Nov 13, 2024

This helps in the very specific case of an observe_value directly or indirectly modifying the signal that is being updated. This creates a busy loop which will not be interrupted by wrapping in asyncio.wait_for. To demonstrate, added
test_observe_value_times_out_with_no_external_task

Originally discussed in DiamondLightSource/dodal#897

This helps in the very specific case of an observe_value directly
or indirectly modifying the signal that is being updated. This
creates a busy loop which will not be interrupted by wrapping
in asyncio.wait_for.  To demonstrate, added
test_observe_value_times_out_with_no_external_task
Copy link
Contributor

@DominicOram DominicOram left a comment

Choose a reason for hiding this comment

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

I agree that I'm still not 100% convinced that this will fix our issue but:

  • It does no harm
  • The tests are great because they match our actual usecases

I would be happy to have this merged instead of DiamondLightSource/dodal#897 and then revisit if we see it again on the beamline

@coretl coretl merged commit 2d4b12b into main Nov 14, 2024
24 checks passed
@coretl coretl deleted the observe-yield branch November 14, 2024 13:50
ZohebShaikh pushed a commit that referenced this pull request Nov 14, 2024
This helps in the very specific case of an observe_value directly
or indirectly modifying the signal that is being updated. This
creates a busy loop which will not be interrupted by wrapping
in asyncio.wait_for.  To demonstrate, added
test_observe_value_times_out_with_no_external_task
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.

2 participants