Incorrect return value on deserializing JointTrajPt

[gonzalocasas]

I am not entirely sure, but this line looks like a bug to me:

https://github.com/ros-industrial/industrial_core/blob/a588028a74a654af3dc06803b9cf6031d3b960e8/simple_message/src/joint_traj_pt.cpp#L111

It will return true even if the duration failed to load on the buffer. I don't know if this is for some reason an intended behavior, but it does feel more like a copy&paste bug. In that case, the fix should probably be to flatten this very nested construct to avoid this kind of bugs from creeping in.

[gavanderhoorn]

That does look like it shouldn't be there, as rtn is now set to true regardless of whether loading succeeded.

---

Edit:

In that case, the fix should probably be to flatten this very nested construct to avoid this kind of bugs from creeping in.

as to the ladder programming: yes, I'm not a fan of that either, but "the fix" would be to remove that line.

Refactoring the conditionals has the potential to introduce new bugs, so should be done separately.

[gavanderhoorn]

Impact of this is rather low btw, as JointTrajPt::load(..) is only used when deserialising a JointTrajPt from an incoming SimpleMessage connection. JointTrajPts are typically only serialised by simple_message, as server implementations are generally written in the OEM's language.

[gonzalocasas]

as to the ladder programming: yes, I'm not a fan of that either, but "the fix" would be to remove that line.

Fair enough. I'll send a PR for the fix.