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

Waypoints don't support mission type #1157

Closed
fnoop opened this issue Jan 22, 2019 · 8 comments · Fixed by #1558
Closed

Waypoints don't support mission type #1157

fnoop opened this issue Jan 22, 2019 · 8 comments · Fixed by #1558

Comments

@fnoop
Copy link
Contributor

fnoop commented Jan 22, 2019

Issue details

Waypoints don't support mission types (fence and rally), it is hardcoded for MAV_MISSION_TYPE = MISSION:
#957
415b789

It looks like the waypoint message needs to be extended for the new field?
http://docs.ros.org/api/mavros_msgs/html/msg/Waypoint.html

And then mavros needs to be updated to allow this field to be queried/set as per the other fields.

MAVROS version and platform

Mavros: 0.28
ROS: Melodic
Ubuntu: 18.04

Autopilot type and version

[ x ] ArduPilot
[ x ] PX4 (presumably)

@vooon
Copy link
Member

vooon commented Jan 22, 2019

No, i think it is useless in Waypoint itself.
More likely that it may be used in Push/Pull services, to manipulate with this lists.

Or i misunderstood spec, and this points is part of whole mission (single list)?

@SamuelDudley
Copy link

SamuelDudley commented Jan 22, 2019

@vooon My understanding is in line with yours in that while the same message is used for waypoint, rally and fence communication ( e.g. https://mavlink.io/en/messages/common.html#MISSION_ITEM ) they represent discreet functionality.
So perhaps two additional push/pull services should be created (similar to the waypoint system) but with hard coded MAV_MISSION_TYPE = 1 for the fence point service and MAV_MISSION_TYPE = 2 for the rally point service (similar to how the value was hardcoded here:415b789) ?

@vooon
Copy link
Member

vooon commented Jan 23, 2019

@SamuelDudley perhaps. Maybe it is better to make basic waypoint io base class, and then do 3 plugins for each MISSION_TYPE.
Usual mission have some additional features, like tracking item change etc. But basic exchange should be same.

@Charlie-Burge
Copy link
Contributor

Hi @vooon I have had a go at implementing this here: https://github.com/Charlie-Burge/mavros/tree/feature/rally-points.

I was initially adding rallypoints but it became clear quickly that adding functionality to send geofences wasn't hard to add beyond this. Does this implementation agree with your comment above?

@vooon
Copy link
Member

vooon commented Mar 26, 2021

@Charlie-Burge yes, that's close to what i want to see.

A few things to change:

  1. i think mission_protocol_base.{h,cpp} would be more descriptive
  2. likely it should be part of plug-in library rather than node lib
  3. base class should implement only protocol part, not ROS interface, because i suppose it might be good to have more specific interface.
  4. also note that logging all of that calls as a wp may be error prone.

@Charlie-Burge
Copy link
Contributor

@vooon I believe I have managed to address your comments above in my latest commit - would it be best for me to submit a pull request to review in further detail?

@vooon
Copy link
Member

vooon commented Mar 29, 2021

@Charlie-Burge yes, please do it.

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

Successfully merging a pull request may close this issue.

4 participants