Rework the period standard deviation calculation in Sta… #1361
Conversation
| msg.period_stddev += ros::Duration(t.toSec() * t.toSec()); | ||
| try | ||
| { | ||
| msg.period_stddev += ros::Duration(t.toSec() * t.toSec()); |
There was a problem hiding this comment.
The result of this temporary result might clearly exceeding the value range of 32 bit. But in the following lines it is divided and then the sqrt is being computed which would bring the value back into a valid range.
Therefore I would suggest to way this value is being computed rather than catching the error and skipping the input.
There was a problem hiding this comment.
Agreed. I will rework this.
| { | ||
| msg.period_stddev = ros::Duration(period_stddev); | ||
| } | ||
| catch(std::runtime_error& e) |
There was a problem hiding this comment.
With the updated math in which cases do you expect this to happen?
There was a problem hiding this comment.
Short answer is never :) The slightly longer mathematial answer that I worked out for my own curiosity is that in a scenario where there are 4 samples where the first 3 arrive within nanoseconds of each other (effectively making the first 2 periods 0) and the 4th arrives much later, the 4th sample would have to arrive 5579326178 seconds or 177 years later. And I won't be around then to deal with the exception. Thanks for pointing this out, I will remove the guard.
… at the end. Fixes #1360
deng02
changed the title
|
Thank you for the patch. The CI failure on Melodic is due to a know compiler warning which requires a new pluginlib release. So merging anyway. |
…tisticsLoggger.
Fixes #1360