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

Warn if Timer frequency is not met #1126

Closed
tonynajjar opened this issue May 11, 2023 · 6 comments
Closed

Warn if Timer frequency is not met #1126

tonynajjar opened this issue May 11, 2023 · 6 comments

Comments

@tonynajjar
Copy link
Contributor

tonynajjar commented May 11, 2023

Feature request

Feature description

How about adding a warning if the timer frequency is not met? That would be useful for the user to either decrease the frequency or optimize the callback.

Implementation considerations

In case the timer is implemented with a ReentrantCallback group, another instance of the callback would always fire; we should decide if we want to warn nevertheless if the first instance of the callback takes longer than expected to complete 🤔

@fujitatomoya
Copy link
Collaborator

i think that would be helpful.

How about adding a warning if the timer frequency is not met?

required accuracy is really dependent on user application. for doing this, i think we do need to ask application about acceptable jitter or something like that to the timer?

my concern is there would be many warning messages. i think there are many possibilities that can cause this warning, executor becomes busy, system is busy and so on. to avoid having many warnings, probably it can print just once when the requirement does not match. and then we are not sure how often it happens, which is really good information if we want to tune the timer frequency. we can have time window for warning message, but it gives more complexity for the timer?

sorry, i just left my note above, i really do not have good idea. i would like to ask for comments on this.

@apockill
Copy link
Contributor

I'm all for more visibility when it comes to logging, but I wouldn't want to add more complexity or compute to the system.

What if before a timer callback gets called, a check is performed to see if the (time - last_time_run) > intended_delay, and log there? It would be simple to implement and maintain, and give some transparency to users.

I also agree logging once might be a good idea.

@tonynajjar
Copy link
Contributor Author

cross-linking as this seems related: ros2/rclcpp#2343

@fujitatomoya
Copy link
Collaborator

@tonynajjar thanks, i was going to do that, but dropped it from my radar. we need to implement it in rclpy as counterpart of ros2/rclcpp#2343

@fujitatomoya
Copy link
Collaborator

@tonynajjar @apockill if you can review #1292, that would be appreciated!

@fujitatomoya
Copy link
Collaborator

@apockill @tonynajjar #1292 has been merged in rclpy, so that we can get the TimerInfo in the application timer callback. i will go ahead to close this one.

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

No branches or pull requests

3 participants