Add latest time zero-order-hold sync policy #73

andermi · 2022-05-13T17:19:27Z

Rate logic added to #38.
Computes rates of received messages and pivots on fastest topic; essentially, upsamples slower topics with a zero-order hold.

Signed-off-by: Michael Anderson <anderson@mbari.org>

clalancette

I've left a few comments inline.

Besides that, it might be nice to add in some documentation to the top of latest_time.h that describes briefly what this sync policy does, and how to use it.

include/message_filters/sync_policies/latest_time.h

test/test_latest_time_policy.cpp

Co-authored-by: Chris Lalancette <clalancette@gmail.com>

Signed-off-by: Michael Anderson <anderson@mbari.org>

andermi · 2022-05-16T22:14:14Z

Thanks @clalancette ! I added some some documentation at the top of the header.

chapulina

The functionality works for me. I have some questions and suggestions. Take it all with a grain of salt since it's my first time touching this repo 😄

include/message_filters/sync_policies/latest_time.h

test/test_latest_time_policy.cpp

include/message_filters/sync_policies/latest_time.h

Co-authored-by: Louise Poubel <louise@openrobotics.org>

…ne is selected Signed-off-by: Michael Anderson <anderson@mbari.org>

…ndermi/message_filters into andermi/latest_time_zoh_policy

Signed-off-by: Michael Anderson <anderson@mbari.org>

test/test_latest_time_policy.cpp

Signed-off-by: Michael Anderson <anderson@mbari.org>

chapulina

Thanks for iterating, LGTM! I'd appreciate a review from one of the maintainers though.

clalancette

Overall, looks pretty good to me. I've left a few things inline that I think we should address.

include/message_filters/sync_policies/latest_time.h

Co-authored-by: Chris Lalancette <clalancette@gmail.com>

…igs; other PR comments Signed-off-by: Michael Anderson <anderson@mbari.org>

andermi · 2022-06-06T22:33:34Z

@clalancette I think/hope I've addressed your comments.

clalancette

All right, one last pass. Once these things are fixed, we'll be ready to run CI on it.

include/message_filters/sync_policies/latest_time.h

test/test_latest_time_policy.cpp

Signed-off-by: Michael Anderson <anderson@mbari.org>

clalancette

Looks good to me! The next step here is to run CI, but we are currently having some infrastructure issues with CI. Once those are resolved I'll do the CI run.

clalancette · 2022-06-08T12:28:09Z

CI:

Linux
Linux-aarch64
Windows

andermi · 2022-06-08T18:56:41Z

Windows

ugh... timing in windows is unstable in general 😝 . I think I can fix it.

Signed-off-by: Michael Anderson <anderson@mbari.org>

andermi · 2022-06-08T19:25:29Z

I actually did find a small intermittent bug

clalancette · 2022-06-09T16:36:03Z

Let's run Windows CI again to see where we are at: * Windows

chapulina

It looks like there are some Windows warnings being introduced by this PR:

https://ci.ros2.org/job/ci_windows/17217/msbuild/new/#issuesContent

test/test_latest_time_policy.cpp

quarkytale · 2022-07-12T21:50:28Z

Retriggered CI

Linux
Linux-aarch64
Windows

quarkytale · 2022-07-28T20:56:39Z

This is currently blocked by timing issue in windows CI.
There are two EMA filters tuned for Linux:

mean rate for incoming message
mean error from 1 which is similar to stddev

In Linux, a threshold of 10 * EMA#2 is used for detecting if the message rate changed. On windows, the rate change is not being detected the same way.

Signed-off-by: Michael Anderson <anderson@mbari.org>

andermi · 2022-08-12T03:44:13Z

@clalancette, @quarkytale I gave it another shot... I spun up ros2 dev on my windows machine.
The test passes using sim /clock to control the test timing

quarkytale · 2022-08-15T21:26:30Z

Retriggered Windows CI

andermi · 2022-08-16T00:16:45Z

Looks like compiler warnings causing unstable. Working through them now.

Signed-off-by: Michael Anderson <anderson@mbari.org>

andermi · 2022-08-16T00:28:25Z

@quarkytale would you mind running another one? I fixed the 7 msbuild warnings related to conversion from size_t. Can you see anything else to fix first?

quarkytale · 2022-08-16T04:15:23Z

I'm not sure if the warnings were an issue, but good to have them cleared. Seems like some tests are being skipped, that might be the reason for unstable status. Other things that stood out to me:

[WARNING] [python.exe-3]: 'SIGINT' sent to process[python.exe-3] not supported on Windows, escalating to 'SIGTERM'
[-ERROR-] IO exception has been thrown: java.nio.file.NoSuchFileException: C:\ci\ws\src\ros2\message_filters\include\message_filters\sync_policies\latest_time.h

Let's see if any of these persist:

quarkytale

LGTM and CI is green!

adityapande-1995 · 2023-03-09T21:23:25Z

https://github.com/Mergifyio backport humble

mergify · 2023-03-09T21:23:29Z

backport humble

✅ Backports have been created

#89 Add latest time zero-order-hold sync policy (backport #73) has been created for branch humble

* adding latest time zoh policy Signed-off-by: Michael Anderson <anderson@mbari.org> * remove commented code * Update include/message_filters/sync_policies/latest_time.h Co-authored-by: Chris Lalancette <clalancette@gmail.com> * updates from PR comments Signed-off-by: Michael Anderson <anderson@mbari.org> * cleanup includes Signed-off-by: Michael Anderson <anderson@mbari.org> * adding documentation Signed-off-by: Michael Anderson <anderson@mbari.org> * Update include/message_filters/sync_policies/latest_time.h Co-authored-by: Louise Poubel <louise@openrobotics.org> * address PR comments; add new test for if pivot changes rate and new one is selected Signed-off-by: Michael Anderson <anderson@mbari.org> * adding message rate step change detection and EMA filter reset Signed-off-by: Michael Anderson <anderson@mbari.org> * whitespace Signed-off-by: Michael Anderson <anderson@mbari.org> * whitespace Signed-off-by: Michael Anderson <anderson@mbari.org> * detect if pivot message is late Signed-off-by: Michael Anderson <anderson@mbari.org> * add ChangeRateLeading test Signed-off-by: Michael Anderson <anderson@mbari.org> * whitespace Signed-off-by: Michael Anderson <anderson@mbari.org> * Update include/message_filters/sync_policies/latest_time.h Co-authored-by: Chris Lalancette <clalancette@gmail.com> * add option to take instance of rclcpp::Clock; make magic numbers configs; other PR comments Signed-off-by: Michael Anderson <anderson@mbari.org> * clean up; handle zero period Signed-off-by: Michael Anderson <anderson@mbari.org> * check for late message after avg error is calculated Signed-off-by: Michael Anderson <anderson@mbari.org> * control test timing with sim time Signed-off-by: Michael Anderson <anderson@mbari.org> * fix msbuild compiler warnings Signed-off-by: Michael Anderson <anderson@mbari.org> Signed-off-by: Michael Anderson <anderson@mbari.org> Co-authored-by: Chris Lalancette <clalancette@gmail.com> Co-authored-by: Louise Poubel <louise@openrobotics.org> (cherry picked from commit e0055f3)

* adding latest time zoh policy Signed-off-by: Michael Anderson <anderson@mbari.org> * remove commented code * Update include/message_filters/sync_policies/latest_time.h Co-authored-by: Chris Lalancette <clalancette@gmail.com> * updates from PR comments Signed-off-by: Michael Anderson <anderson@mbari.org> * cleanup includes Signed-off-by: Michael Anderson <anderson@mbari.org> * adding documentation Signed-off-by: Michael Anderson <anderson@mbari.org> * Update include/message_filters/sync_policies/latest_time.h Co-authored-by: Louise Poubel <louise@openrobotics.org> * address PR comments; add new test for if pivot changes rate and new one is selected Signed-off-by: Michael Anderson <anderson@mbari.org> * adding message rate step change detection and EMA filter reset Signed-off-by: Michael Anderson <anderson@mbari.org> * whitespace Signed-off-by: Michael Anderson <anderson@mbari.org> * whitespace Signed-off-by: Michael Anderson <anderson@mbari.org> * detect if pivot message is late Signed-off-by: Michael Anderson <anderson@mbari.org> * add ChangeRateLeading test Signed-off-by: Michael Anderson <anderson@mbari.org> * whitespace Signed-off-by: Michael Anderson <anderson@mbari.org> * Update include/message_filters/sync_policies/latest_time.h Co-authored-by: Chris Lalancette <clalancette@gmail.com> * add option to take instance of rclcpp::Clock; make magic numbers configs; other PR comments Signed-off-by: Michael Anderson <anderson@mbari.org> * clean up; handle zero period Signed-off-by: Michael Anderson <anderson@mbari.org> * check for late message after avg error is calculated Signed-off-by: Michael Anderson <anderson@mbari.org> * control test timing with sim time Signed-off-by: Michael Anderson <anderson@mbari.org> * fix msbuild compiler warnings Signed-off-by: Michael Anderson <anderson@mbari.org> Signed-off-by: Michael Anderson <anderson@mbari.org> Co-authored-by: Chris Lalancette <clalancette@gmail.com> Co-authored-by: Louise Poubel <louise@openrobotics.org> (cherry picked from commit e0055f3) Co-authored-by: andermi <manders9@jhu.edu>

andermi added 2 commits May 13, 2022 10:04

adding latest time zoh policy

852e54e

Signed-off-by: Michael Anderson <anderson@mbari.org>

remove commented code

cf4e7b7

andermi marked this pull request as draft May 13, 2022 17:22

andermi marked this pull request as ready for review May 13, 2022 17:22

chapulina self-requested a review May 13, 2022 17:38

clalancette requested changes May 16, 2022

View reviewed changes

andermi and others added 4 commits May 16, 2022 11:55

Update include/message_filters/sync_policies/latest_time.h

7cbc152

Co-authored-by: Chris Lalancette <clalancette@gmail.com>

updates from PR comments

c90bc60

Signed-off-by: Michael Anderson <anderson@mbari.org>

cleanup includes

65f63d7

Signed-off-by: Michael Anderson <anderson@mbari.org>

adding documentation

be580c1

Signed-off-by: Michael Anderson <anderson@mbari.org>

andermi requested a review from clalancette May 16, 2022 19:46

chapulina reviewed May 17, 2022

View reviewed changes

andermi and others added 3 commits May 17, 2022 12:39

Update include/message_filters/sync_policies/latest_time.h

6e7174d

Co-authored-by: Louise Poubel <louise@openrobotics.org>

address PR comments; add new test for if pivot changes rate and new o…

d602a47

…ne is selected Signed-off-by: Michael Anderson <anderson@mbari.org>

Merge branch 'andermi/latest_time_zoh_policy' of https://github.com/a…

1dbdf65

…ndermi/message_filters into andermi/latest_time_zoh_policy

andermi requested a review from chapulina May 18, 2022 00:34

andermi added 3 commits May 18, 2022 11:39

adding message rate step change detection and EMA filter reset

72fad82

Signed-off-by: Michael Anderson <anderson@mbari.org>

whitespace

3b068af

Signed-off-by: Michael Anderson <anderson@mbari.org>

whitespace

f1b973c

Signed-off-by: Michael Anderson <anderson@mbari.org>

chapulina reviewed May 18, 2022

View reviewed changes

test/test_latest_time_policy.cpp Outdated Show resolved Hide resolved

andermi added 3 commits May 18, 2022 17:23

detect if pivot message is late

c834584

Signed-off-by: Michael Anderson <anderson@mbari.org>

add ChangeRateLeading test

d15f2de

Signed-off-by: Michael Anderson <anderson@mbari.org>

whitespace

dcac3bd

Signed-off-by: Michael Anderson <anderson@mbari.org>

chapulina approved these changes Jun 2, 2022

View reviewed changes

clalancette requested changes Jun 3, 2022

View reviewed changes

andermi and others added 2 commits June 3, 2022 12:56

Update include/message_filters/sync_policies/latest_time.h

51d5343

Co-authored-by: Chris Lalancette <clalancette@gmail.com>

add option to take instance of rclcpp::Clock; make magic numbers conf…

c61a951

…igs; other PR comments Signed-off-by: Michael Anderson <anderson@mbari.org>

andermi requested a review from clalancette June 6, 2022 22:33

clalancette requested changes Jun 7, 2022

View reviewed changes

clean up; handle zero period

1e5e7cb

Signed-off-by: Michael Anderson <anderson@mbari.org>

clalancette approved these changes Jun 7, 2022

View reviewed changes

check for late message after avg error is calculated

869b31e

Signed-off-by: Michael Anderson <anderson@mbari.org>

chapulina reviewed Jun 15, 2022

View reviewed changes

test/test_latest_time_policy.cpp Outdated Show resolved Hide resolved

audrow changed the base branch from master to rolling June 28, 2022 14:19

clalancette assigned quarkytale Jul 28, 2022

control test timing with sim time

b02300f

Signed-off-by: Michael Anderson <anderson@mbari.org>

andermi requested review from chapulina and clalancette August 12, 2022 03:44

fix msbuild compiler warnings

86c7440

Signed-off-by: Michael Anderson <anderson@mbari.org>

quarkytale approved these changes Aug 16, 2022

View reviewed changes

gbiggs merged commit e0055f3 into ros2:rolling Aug 16, 2022

mergify bot mentioned this pull request Mar 9, 2023

Add latest time zero-order-hold sync policy (backport #73) #89

Merged

Add latest time zero-order-hold sync policy #73

Add latest time zero-order-hold sync policy #73

Uh oh!

Conversation

andermi commented May 13, 2022

Uh oh!

clalancette left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

andermi commented May 16, 2022

Uh oh!

chapulina left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

chapulina left a comment

Choose a reason for hiding this comment

Uh oh!

clalancette left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

andermi commented Jun 6, 2022

Uh oh!

clalancette left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

clalancette left a comment

Choose a reason for hiding this comment

Uh oh!

clalancette commented Jun 8, 2022

Uh oh!

andermi commented Jun 8, 2022

Uh oh!

andermi commented Jun 8, 2022

Uh oh!

clalancette commented Jun 9, 2022

Uh oh!

chapulina left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

quarkytale commented Jul 12, 2022

Uh oh!

quarkytale commented Jul 28, 2022

Uh oh!

andermi commented Aug 12, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

quarkytale commented Aug 15, 2022

Uh oh!

andermi commented Aug 16, 2022

Uh oh!

andermi commented Aug 16, 2022

Uh oh!

andermi commented Aug 12, 2022 •

edited

Loading

mergify bot commented Mar 9, 2023 •

edited

Loading