Skip to content
This repository was archived by the owner on May 31, 2025. It is now read-only.

Fix #2249: Topic statistics do not handle well ROS time jumping back#2250

Closed
peci1 wants to merge 2 commits intoros:noetic-develfrom
peci1:fix-stats-time-jump-noetic
Closed

Fix #2249: Topic statistics do not handle well ROS time jumping back#2250
peci1 wants to merge 2 commits intoros:noetic-develfrom
peci1:fix-stats-time-jump-noetic

Conversation

@peci1
Copy link
Copy Markdown
Contributor

@peci1 peci1 commented Jun 6, 2022

First, I'm submitting a failing test.

It makes 4 repetitions of receiving 10000 messages, each time resetting ROS time from 1.0 onwards. First repetition is okay. If #2249 is not fixed, the following iterations will take considerably longer because the internal lists are never cleared.

@peci1 peci1 force-pushed the fix-stats-time-jump-noetic branch from ef5cbaa to 025980c Compare June 6, 2022 02:06
@peci1
Copy link
Copy Markdown
Contributor Author

peci1 commented Jun 6, 2022

And the fix comes.

@peci1
Copy link
Copy Markdown
Contributor Author

peci1 commented Jun 6, 2022

Is there actually reason to copy the whole stats structure twice in each callback? Wouldn't it be better to modify it directly via an auto&? Could the reason be keeping the lists consistent even in usage by different threads? But that would mean some data would be lost if more threads were overwriting the stats structure...

@peci1
Copy link
Copy Markdown
Contributor Author

peci1 commented Apr 9, 2023

@mjcarroll @sloretz friendly ping

@sloretz
Copy link
Copy Markdown
Contributor

sloretz commented Apr 25, 2025

Thank you for the PR!

ROS Noetic will reach end-of-life on May 31st, 2025. Every change comes with a risk of introducing regressions, and there isn't much time left to fix them. To make sure this PR doesn't introduce any regressions please:

  • Describe how you tested this change
  • Recruit at least one more person to review this PR and try it out on their system

@peci1
Copy link
Copy Markdown
Contributor Author

peci1 commented May 4, 2025

This PR comes with a unit test that fails without this PR and succeeds with it. The test does something that should be completely valid - having topic statistics enabled, receiving lots of messages and then resetting ROS time back.

@sloretz
Copy link
Copy Markdown
Contributor

sloretz commented May 31, 2025

ROS 1 is end-of-life (EOL) as of today, May 31st 2025. I am archiving this repository because:

  • it only supports ROS 1
  • it isn't needed anymore in ROS 2

If you still rely on ROS 1, read this page to learn about your options.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants