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

subt.main: add TimeoutMonitor #373

Draft
wants to merge 3 commits into
base: master
Choose a base branch
from
Draft

Conversation

zbynekwinkler
Copy link
Member

Implement part of #369 by providing TimeoutMonitor class. Example usage is in test_subt.py. Having this context manager allows adding timeout to any function that periodicaly calls update() inside.

@zbynekwinkler zbynekwinkler requested a review from m3d February 22, 2020 20:38
@zbynekwinkler zbynekwinkler added this to the SubT Urban milestone Feb 22, 2020
subt/main.py Outdated Show resolved Hide resolved
subt/main.py Outdated
pass


class TimeoutMonitor:
Copy link
Member

Choose a reason for hiding this comment

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

I would vote for separate file (main.py is too big already) ... and maybe it is generic enough that we want it in osgar.lib? Or monitors??

subt/main.py Outdated

def __exit__(self, exc_type, exc_val, exc_tb):
self.robot.unregister(self.handle)
if exc_val is not None:
Copy link
Member

Choose a reason for hiding this comment

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

I would check that the type matches to TimeoutEx

@zbynekwinkler zbynekwinkler force-pushed the feature/timeout_monitor branch from ec3f108 to 9385c25 Compare February 23, 2020 18:43
@zbynekwinkler zbynekwinkler removed this from the SubT Urban milestone Feb 25, 2020
@zbynekwinkler zbynekwinkler self-assigned this Feb 29, 2020
@zbynekwinkler
Copy link
Member Author

My plan with this PR is to add usage of the TimeoutMonitor. I'd like to test it in Virtual world, but for that I need to do some fixes first. So I would mark this "on hold" until then.

@zbynekwinkler zbynekwinkler marked this pull request as draft April 9, 2020 09:24
@zbynekwinkler zbynekwinkler force-pushed the feature/timeout_monitor branch from 9385c25 to 476bbc6 Compare April 9, 2020 09:46
@zbynekwinkler zbynekwinkler marked this pull request as ready for review April 20, 2020 11:38
@zbynekwinkler zbynekwinkler marked this pull request as draft April 20, 2020 12:00
@m3d
Copy link
Member

m3d commented May 24, 2020

I see that we already had monitors implemented! I also see implementation of your comment where you catch exceptions in the exit

@zbynekwinkler
Copy link
Member Author

Well... yet, but you didn't want to merge it until it is used as well. I didn't get to use it since I was hunting the bigger problems in virtual. So it is waiting here until I get to it again.

@m3d
Copy link
Member

m3d commented Dec 18, 2020

I needed monitors for external "locomotive" project, where I use osgar pypi installation - maybe it would be nice if this would be directly part of osgar and not SubT? Also Moon needed monitors ...maybe part of larger refactoring, including Node? (it requires/expects register() and also call in update(), right?)

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

Successfully merging this pull request may close these issues.

2 participants