[JTC] Fix race condition & interpolation bug #410
Closed
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR addresses a few bugs:
new_external_msgwhen reading fromtraj_msg_external_point_ptr_.readFromRT();and an active goal when reading from*rt_active_goal_.readFromRT();within the same update cycle. However I noticed based on a project I'm working on that after a new goal comes in, sometimes whentraj_msg_external_point_ptr_.readFromRT();is called, it still returns the old trajectory for a cycle (most likely since the new one is still being written to the RT thread from the nonRT thread. However, because*rt_active_goal_.readFromRT();is called later within the update cycle, it actually returns that there is an active goal causing the JTC to return success immediately since the trajectory still cached within the JTC has indeed been completed (now for a second time). See log output below. Note I added the "new message received" log to note when thenew_external_msgwithin the update callback had actually been changed. That log should always occur between the logs for "Goal request accepted!" and "Goal reached, success" but here, you can see it occurred after both. Another sanity check is that the time difference between "Goal request accepted!" and "Goal reached, success!" is super small (like less than 0.1ms) implying that the goal returned success immediately.This PR therefore moves when we read from
*rt_active_goal_.readFromRT();to be at the same point in time as when we read fromtraj_msg_external_point_ptr_.readFromRT();(which makes sense I think just in general in that it's probably good practice to query all relevant values from the RT thread at the same point in time). This fixes the race condition since I did not see this problem come up again when testing my project.Due to moving when we query
*rt_active_goal_.readFromRT();to be earlier than it originally was in the update callback I noticed that some of the JTC tests were failing. Turns out it was because after a new goal came in,new_external_msghad been updated but theactive_goalhad not yet been (though it would have if it was called later in the code in the same cycle). To handle this, I added a check to see if there is a mismatch between when querying the active goal status earlier in the code and later in the code and if so, to check whether thenew_external_msghad also been updated that cycle. If thenew_external_msghad not been updated that cycle and is still the same as thecurrent_external_msg(i.e. the old one), then we skip that cycle. Otherwise we can continue the cycle since we have already updated theactive_goalstatus when we checked the second time.There is an interpolation bug that occurs in the following case:
last_commanded_state_is prepended when a new trajectory comes in)This can lead to cases where the commanded state can be -PI but the feedback the joint gives is PI due to encoder resolution. When a new trajectory comes, it usually is based on the feedback (so PI in this case). When the trajectory is prepended with the last commanded state, you then can encounter the JTC trying to interpolate between -PI and PI really really quickly causing faults on the hardware. This is because interpolation does not currently take into account the shortest-angle. This PR fixes this.