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

Add an OdometryWithAcceleration message #87

Open
wants to merge 2 commits into
base: jade-devel
Choose a base branch
from
Open

Conversation

tfoote
Copy link
Member

@tfoote tfoote commented Jul 12, 2016

As needed by REP 147 discussion: Pull request and discourse thread

@jack-oquin
Copy link
Member

Where was OdometryWithAcceleration discussed? I can't find it.

@tfoote
Copy link
Member Author

tfoote commented Jul 12, 2016

@TSC21
Copy link

TSC21 commented May 13, 2018

@tfoote is this going to be merged soon? This is an interesting approach that we probably want to explore on MAVROS.

@tfoote
Copy link
Member Author

tfoote commented May 18, 2018

I would like to merge this soon. I think it would be rushed to push this out before the melodic release as I'd like to cross reference it with the update for REP 145 from #101 though looking at that now. It's really for the raw measurements, whereas the reported odometry should be fully fused.

But since this is a pure addtion we can add it out of cycle as well as can backport it so I'm not going to push to get it into melodic before the release next week.

@TSC21 If you are thinking about picking it up. I'd love it if you would copy the message and try it out locally to get feedback on the structure. And if it suits your needs that would be great to know that the message in this format is working.

@TSC21
Copy link

TSC21 commented May 18, 2018

@tfoote this is of most interest for us developers as we are aiming to push it further for interfacing ROS with PX4 (if you are interested, have a look of what is being done in PX4/PX4-Autopilot#9301). This would probably be tested, in a first stage, on simulation, and then move forward with its support both ROS and PX4 estimators. PX4 Pro would probably be the best autopilot to be in the vanguard for testing and pushing this forward.

@tfoote
Copy link
Member Author

tfoote commented May 18, 2018

Thanks for the link. I've subscribed to that ticket. I hope to have some more support for a specific effort to push more standardized messages soon.

@TSC21
Copy link

TSC21 commented Sep 19, 2018

@tfoote any updates regarding this?

tfoote added a commit to osrf/drone_demo that referenced this pull request Dec 20, 2018
@m-naumann
Copy link

m-naumann commented May 22, 2019

We are also using an identical message for quite a while since it's very useful: https://github.com/fzi-forschungszentrum-informatik/automated_driving_msgs/blob/master/msg/MotionState.msg

I would recommend to also clarify the treatment of unknown/incomplete covariance information.

Further, I'd suggest to keep the term twist as it's actual values are in the twist member within the TwistWithCovariance anyway, along with the covariance.

I could provide a PR if you like.

Edit:

I also just saw a bug in line 9 of the new message, it should be AccelWithCovariance instead of twist, and I'd also name it accel as within this message, like above

@m-naumann
Copy link

I just fixed these issues in #146

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.

4 participants