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

Change double comparison to use epsilon instead of 0.0 #236

Closed
wants to merge 1 commit into from

Conversation

ted-miller
Copy link
Collaborator

Regarding #233 (comment)

I don't know if this will have an impact on #233. But it does seem like a potential problem.

@@ -503,7 +503,7 @@ void Ros_ActionServer_FJT_Goal_Complete(GOAL_END_TYPE goal_end_type)
diff = fabs(feedback_FollowJointTrajectory.feedback.desired.positions.data[axis] - feedback_FollowJointTrajectory.feedback.actual.positions.data[axis]);

double posTolerance = g_actionServer_FJT_SendGoal_Request.goal.goal_tolerance.data[axis].position; //user-provided a goal_tolerance
Copy link
Collaborator

@gavanderhoorn gavanderhoorn Apr 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we not check whether g_actionServer_FJT_SendGoal_Request.goal.goal_tolerance.data is of the expected/required size? Otherwise we could be trying to read outside the array.

We might already be doing that before we get here, I haven't checked.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We might already be doing that before we get here, I haven't checked

We're not.

You're correct in that this is another potential issue.

@yai-rosejo mentioned in passing that he's got a branch doing some more thorough checking. I'll let him comment on what he's done.

@gavanderhoorn
Copy link
Collaborator

@ted-miller: I suggest we close this in favour of the more correct fix @yai-rosejo is preparing (#233 (comment))

@ted-miller
Copy link
Collaborator Author

agreed

@ted-miller ted-miller closed this Apr 15, 2024
@gavanderhoorn gavanderhoorn deleted the compare_double branch April 15, 2024 16:50
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 this pull request may close these issues.

2 participants