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

Is there a bug in Latest time policy rates_ initialization? #110

Open
wuchenhaogit opened this issue Jan 21, 2024 · 1 comment · May be fixed by #111
Open

Is there a bug in Latest time policy rates_ initialization? #110

wuchenhaogit opened this issue Jan 21, 2024 · 1 comment · May be fixed by #111
Assignees

Comments

@wuchenhaogit
Copy link

wuchenhaogit commented Jan 21, 2024

In include/message_filters/sync_policies/latest_time.h

rates_ is array of struct Rate. I thought that its index corresponds to channel index. But its initialization in function initialize_rate<i>() doesn't put the struct (including alphas and prev) to index "i". Instead "push_back" is used. As a result, the index of rates_ seems to represent the order of arrival of the first message of each channel. This appears confusing to me. How can I properly set the alphas for each channel?

Here is the source code of initialize_rate():

  template<int i>
  void initialize_rate()
  {
    if (rate_configs_.size() > 0U) {
      double rate_ema_alpha{Rate::DEFAULT_RATE_EMA_ALPHA};
      double error_ema_alpha{Rate::DEFAULT_ERROR_EMA_ALPHA};
      double rate_step_change_margin_factor{Rate::DEFAULT_MARGIN_FACTOR};
      if (rate_configs_.size() == RealTypeCount::value) {
        std::tie (
          rate_ema_alpha,
          error_ema_alpha,
          rate_step_change_margin_factor) = rate_configs_[i];
      } else if (rate_configs_.size() == 1U) {
        std::tie (
          rate_ema_alpha,
          error_ema_alpha,
          rate_step_change_margin_factor) = rate_configs_[0U];
      }
      rates_.push_back(
        Rate(
          ros_clock_->now(),
          rate_ema_alpha,
          error_ema_alpha,
          rate_step_change_margin_factor));
    } else {
      rates_.push_back(Rate(ros_clock_->now()));
    }
  }
@andermi
Copy link
Contributor

andermi commented Jan 23, 2024

Confirmed. I'll submit a PR. Thanks for finding this!

@andermi andermi linked a pull request Jan 23, 2024 that will close this issue
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

Successfully merging a pull request may close this issue.

3 participants