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

obstacle_distance: measurament units #949

Closed
wants to merge 1 commit into from

Conversation

mrivi
Copy link
Contributor

@mrivi mrivi commented Feb 19, 2018

min_distance and max_distance are uint16_t in the obstacle_distance mavlink message. It makes more sense to keep them as units and transform them into cm where the message gets used.
I'll update also the mavlink message description. @TSC21 what do you think?

FYI @ChristophTobler

@TSC21
Copy link
Member

TSC21 commented Feb 19, 2018

Works for me. Please update Mavlink side so I can merge it. Also doesn't it sound better one unit per cm?

@mrivi
Copy link
Contributor Author

mrivi commented Feb 19, 2018

@TSC21 mavlink PR done :)

@vooon
Copy link
Member

vooon commented Feb 19, 2018

Nope! You violate LaserScan.msg docs.

@vooon vooon closed this Feb 19, 2018
@vooon
Copy link
Member

vooon commented Feb 19, 2018

Hmm, LaserScan.ranges also should be meters. Bug!

@TSC21
Copy link
Member

TSC21 commented Feb 19, 2018

@vooon I get your point about the docs, but what bug are you refering to?
@mrivi, @vooon is right, though we can keep the logic by dividing by 100 and rounding the values of the distances so you have units per cm.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants