Missing messages/field from first release

[narcispr]

Hi,
first of all I want to say that, in general, the changes for me are great.
However, there are three things I would like to discuss:

Auto descriptive fields:

Bool6Axis and RPY messages are not standard messages but they help to make the code more clear and auto descriptive (ok, and larger too!). I will keep them instead of: bool[6] and geometry_msgs/Vector3.

Missing field:

Our architecture was using intensively the filed GoalDescriptor. I think that at least it is important to keep the requester and priority fields. When some node sends a request, it is important to know who is it and which is its priority.

Missing message:

For me, there is only one missing message that should be included for completeness: WorldWaypointReq. If there are the velocity and the force requests why not the position one?

Thank you, and good job!

[bmagyar]

Hi Narcis,

Thanks for your feedback! Let's continue the discussion and keep refining this package to our needs. Please keep in mind that while this is an attempt at standardizing this package, it is unlikely that any of us will be able to use it without modifications to our codebase.

Bool6Axis and RPY messages:

These are not related to the underwater domain and I doubt they would qualify anywhere else. RPY may have a chance to make it to geometry_msgs but since ROS is using quaternions mostly for rotation representation I'm not sure.
Bool6Axis is indeed handy but we should come up with a more general solution then and add it somewhere else.

GoalDescriptor field

This felt like a remnant of an old generated action descriptor. Do you have suggestions to have a similar mechanism but in a slightly different way than it was? Just to see whether we can improve the message so it fits your use better (we are not using it at the moment).

WorldWaypointReq message:

This message could do with a little bit of cleaning in my opinion. The comment related to the Hz information is platform dependent, altitude_mode could be renamed to self-explain the comment above it.

max_speed_percent could perhaps be a full 6dof limit? I understand that the current one is tailored to a top-down view map use-case, but would a more general version be useful? So that you could limit angular velocity (to aid sensing) or Z-travel (pressure change for instance)?

I pasted the message below for reference:

# World frame (absolute) waypoint request to pilot.
# A new waypoint request should contain a different goal.id
# (incremented) from previous requests. This same message should then
# be re-sent at 5-10 Hz.

Header header

# Common waypoint details
GoalDescriptor goal

# If true, maintain Z position relative to altitude, otherwise depth.
bool altitude_mode

auv_msgs/NED position
float32 altitude
RPY orientation

# Axes of control to disable, in body frame.
Bool6Axis disable_axis

# Tolerances are in body frame, zero indicates pilot default should be used.
geometry_msgs/Vector3 position_tolerance
RPY orientation_tolerance

# Capping speed at a percentage of maximum speed in x-y plane allowed
int8 max_speed_percent

[narcispr]

Hi, I totally understand that a major review of all previous code will be necessary to use the new auv_msgs. No problem on that.

About Bool6Axis and RPY I see that they are not AUV specific. Maybe we can replace the Bool6Axis by a bool[6] and add some #define like structure?
uint8 x=0
uint8 y=1
uint8 z=2
uint8 pitch=3
uint8 roll=4
uint8 yaw=5

About the RPY message, probably the most correct solution would be to replace them by quaternions but this implies lots of modifications! Sure we can survive with a geometry_msgs/Vector3 message ;)

Regarding the GoalDescriptor, this is the most critical part for us. Including a priority and a requester filed on all request messages is paramount in our architecture. For me, these request messages are quiet different of action_libs. They allow you to combine multiple request from multiple requesters (you can't do that with an action_lib) but then, it is important to know who is requesting and whit which priority.

Finally, sure that the WorldWaypointReq message must be modified as the others. About the max_speed_percent it is a feature that I've never used but if you want to change it, probably a simple 6DOF speed vector will be a good solution.

Thanks!

[bmagyar]

Hiya,

Bool6Axis

This is a though one... If I'd need to pick from the define-like structure and the original message, then I'd just bring that back...
Any thoughts? @dulanad @mvaldenegro @decabyte

RPY

So are you ok with this? I adapted our code and the changes were not significant at all, plus the readability of the code is pretty much the same, see this diff:

•    ns.orientation.roll = pos[3]
  

•    ns.orientation.pitch = pos[4]
  

•    ns.orientation.yaw = pos[5]
  

•    ns.orientation.x = pos[3]
  

•    ns.orientation.y = pos[4]
  

•    ns.orientation.z = pos[5]
  

GoalDescriptor

We are open to putting this back to the *Req messages. Would you prefer to keep it the way it is or to remove some unused fields or add some that is needed?

WorldWaypointReq

Preferences? The 6DOF speed vector sounds like a good idea!

[dulanad]

Hi,
as reply to the last comment, here are my opinions:

The replacement of Bool6Axis with bool[6] seems OK since we rarely access these fields directly.

Regarding RPY, x,y,z instead of r,p,y seems also OK . However, if RPY goes away, shouldn't the N/E/D in position also change to x,y,z ? Changing RPY to quaternions would require code adoption, but for us it would not be a large inconvenience. Personally, I like RPY better as it is more readable.

I do not really use the WorldWaypointReq, so I do not have an opinion on that, but having independent velocity capping for each DoF makes sense.

[henriqueribeiro]

Hello guys,

Here Henrique from IST.

To be honest we don't have an informed opinion about these messages, since we have our own messages. So, all the decisions that you take, will fit for us. Of course, in the near future, and when this package is stable, we will adapt our code to use it but for now we don't have "sensibility" to propose alternatives.

Regarding NavSts:

• Maybe we should add a depth field;
• Why use different type of messages, for instance, body_velocity/seafloor_velocity (geometry_msgs/Point) and orientation/etc.(geometry_msgs/Vector3);

Another question, what if we create a Status message (Sts) for more vehicle-related information? If we already have one for Navigation (NavSts), maybe we should create one common for our AUVs.
Suggestion of some of the fields:

• uint8 gps_mode: I think the standard is 0: Autonomous, 1: RTCM code differential, 2: RTK float, 3: RTK fixed, 5: Extrapolated;
• uint8 imu_status: Some IMUs report their own status;
• uint8 battery_level;
• float64 inside_pressure: to monitor the pressure inside the vehicle;
• float64 inside_temperature: to monitor the temperature inside the vehicle;
• etc..

Regarding AUVs sensor_msgs, someone had ideas about this topic? @narcispr @bmagyar @dulanad

[narcispr]

Hi Henrique! Good to see you around :)
The NavSts already has a depth filed in the NED position and an altitude one.

About the status message that you comment, we have something similar implemented using diagnostic_msgs that are standard and very flexible (you can put whatever you need).

Cheers!

[henriqueribeiro]

The NavSts already has a depth filed in the NED position and an altitude one.

Oops, you are right @narcispr :) . So the only change is to replace geometry_msgs/Point by geometry_msgs/Vector3?

About the status message that you comment, we have something similar implemented using diagnostic_msgs that are standard and very flexible (you can put whatever you need).

Yes, diagnostic_msgs are very good, but the problem is that when you have a large array sometimes is hard to monitor all the variables. But ok, for now this is not a priority.

[tfurf]

Hi all,

Maybe I'm missing something, but why not use the "standard" types as much as possible?

• geometry_msgs/Point instead of auv_msgs/NED?
• no RPY (use geometry_msgs/Quaternion)

For the general status @henriqueribeiro is taking about, I agree with @narcispr , diagnostic_msgs is the standard solution for this. Yes it can be overly verbose, but there are tools for this. The problem with creating a specific message with the enums that you outline is that when you change that list it impacts everyone (maintenance problem).

Also, just for completeness and following the ROS style, NavSts should probably be NavigationStatus, and all the *Req messages should be *Request.

Also the fact that WorldWaypointReq[uest] has the word 'Request' in it makes me think it should be part of an action. I would suggest that (along with the VelocityReq and ForceReq) these are part of the Goal of the WorldWaypoint, BodyVelocity, and BodyForce actions. What is nice then is that if you elect to use an action, it is already defined, and if you would prefer to just use one part of it (like the Goal only) then actionlib generates that for you too to use directly as a message.

Tom (CMRE)

[bmagyar]

Hi people,

Status flags:

I'm generally against keeping these, since they are pretty hardcoded and ambiguous. Truth be told, I only kept them in NavSts because I saw some used it, we don't use it anymore, diagnostics works better.
The same applies to Henrique's other device and status-like flags, I think those can vary a lot based on platforms and generally we shouldn't be gathering those on a single topic (or if yes, that's called diagnostics :))

RPY:

I'm not sure about this. In general I wouldn't mind but I'm a wee bit worried that breaking that much of backward compatibility wouldn't help getting the package adopted. Keep in mind that auv_msgs has been lurking around in different versions in most underwater robotics labs, this is an attempt at unifying those.

Message names:

Makes sense.

Actions:

Not sure on this one. While we understand how it works, other people (or us forgetting about this discussion) may get confused when only half of an action is being used directly on a topic, feels a bit hack-ish to me. The ros wiki doesn't list the generated messages in the docu either, e.g. see here
But maybe we can live with this? Opinions?

Getting feedback for these requests may be useful though despite that it doesn't seem that anyone is using it that way at the moment but I can think of a scenario where a mission execution module would be glad to receive feedback that his waypointrequests are being overridden by an emergency procedure triggered by some failsafe mechanism.

NED:

What I experienced is that using NED helps to separate land coordinate systems from ground coordinate systems. I consider it a type-safe way to avoid mixing them.

[narcispr]

Agree with bmagyar!

[decabyte]

Hi all,
sorry if I pitch in late on this topic. I agree with most of what @narcispr said so far.

Regarding the action's discussion, however, I want to point out that in my opinion actions should not be part of the msg package as they tend to be customised according to what is needed in a specific architecture or project (e.g. see PANDORA, ARROWS, TRIDENT, etc).

[bmagyar]

Resolved in #4