Add window size

[Tacha-S]

I have added a window size to calculate the moving average

• fix Enable setting window size for MovingAverageStatistics  #208

[christophebedard]

Unfortunately, we have to consider multiple things:

1. By default, this should probably behave like it did before, i.e., it should consider all values. The tests are currently failing because this is not the case.
2. Tests should be added to cover the case where a window is used (probably in test/moving_average_statistics/test_moving_average_statistics.cpp).
3. Finally: How would an end user use this? In this case, this code is ultimately used in rclcpp (see rclcpp::topic_statistics::SubscriptionTopicStatistics). From looking at the subscription creation code in rclcpp, there is no way to provide a window_size value.
   • One solution might be to add a window_size option to TopicStatisticsOptions: https://github.com/ros2/rclcpp/blob/9c493c4615aee3b5cbc68e9101b588564869b5dc/rclcpp/include/rclcpp/subscription_options.hpp#L69-L85
   • Then this would be used here: https://github.com/ros2/rclcpp/blob/9c493c4615aee3b5cbc68e9101b588564869b5dc/rclcpp/include/rclcpp/create_subscription.hpp#L56
   • We have to figure out which statistics this should affect.

@emersonknapp do you have any thoughts on this?

[Tacha-S]

I understood points 1 and 2.

I noticed this feature in the controller manager, so I thought the user would create an instance, but it turns out that it can be enabled or disabled through the options when subscribing in rclcpp.

[emersonknapp]

Thanks for making the updates! Looks good to me

[emersonknapp]

Pulls: #210
Gist: https://gist.githubusercontent.com/emersonknapp/48d2d4f9dfc18c1a2c3724f80b2ad482/raw/e0539bcc3241c19ede2ef7af02ad13e11a3121a8/ros2.repos
BUILD args: --packages-above-and-dependencies libstatistics_collector
TEST args: --packages-above libstatistics_collector
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/15705

• Linux
• Linux-aarch64
• Linux-rhel
• Windows

Requests addressed

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Please upload report for BASE (rolling@ec4d751). Learn more about missing BASE report.

Additional details and impacted files

@@            Coverage Diff             @@
##             rolling     #210   +/-   ##
==========================================
  Coverage           ?   22.24%           
==========================================
  Files              ?       38           
  Lines              ?     1425           
  Branches           ?      203           
==========================================
  Hits               ?      317           
  Misses             ?     1105           
  Partials           ?        3           

Flag	Coverage Δ
unittests	22.24% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[christophebedard]

Since we're in the Kilted feature freeze, and since this is part of the ROS Base packages (rclcpp depends on this), we can't merge this until we branch Kilted off of Rolling next Monday.

[christophebedard]

Looks good to me! But like I said we'll have to wait until we create a kilted branch next week before merging this.

[ahcorde]

@christophebedard what's the status of this PR, can we merge it ?

[ahcorde]

Pulls: #210
Gist: https://gist.githubusercontent.com/ahcorde/0114ff033fb17a01ac19807c5d01d27c/raw/79e75aba70670e056cbc42fbd51829ad0bfc1578/ros2.repos
BUILD args: --packages-above-and-dependencies libstatistics_collector
TEST args: --packages-above libstatistics_collector
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/17130

• Linux
• Linux-aarch64
• Linux-rhel
• Windows

[ahcorde]

friendly ping @christophebedard

[ahcorde]

friendly ping @christophebedard

[MichaelOrlov]

@ahcorde, I think this branch has become somewhat outdated. Someone needs to rebase it on top of the latest Rolling and rerun the CI.