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.

Sign up for GitHub

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

Jump to bottom

VTOL - define MAV_TYPE for belly-sitting non tiltrotor #1808

Merged
merged 5 commits into from
Mar 16, 2022
Merged

VTOL - define MAV_TYPE for belly-sitting non tiltrotor #1808

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b04d93c
VTOL - define MAV_TYPE for belly-sitting non tiltrotor
hamishwillee Mar 3, 2022
46b078a
Apply suggestions from code review
hamishwillee Mar 9, 2022
d2f4ccd
Update message_definitions/v1.0/minimal.xml
hamishwillee Mar 9, 2022
0c5f04c
Reorder to allow eventual reuse
hamishwillee Mar 9, 2022
d45f25a
Shift items up one to use types from top
hamishwillee Mar 16, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 8 additions & 8 deletions message_definitions/v1.0/minimal.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,21 +128,21 @@
<description>Onboard companion controller</description>
</entry>
<entry value="19" name="MAV_TYPE_VTOL_DUOROTOR">
<description>Two-rotor VTOL using control surfaces in vertical operation in addition. Tailsitter.</description>
<description>Two-rotor Tailsitter VTOL that additionally uses control surfaces in vertical operation. Note: should be named MAV_TYPE_VTOL_TAILSITTER_DUOROTOR.</description>
Copy link
Contributor

Choose a reason for hiding this comment

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

Would we break anything by renaming it directly instead of just remarking that it should be renamed?

Copy link
Collaborator Author

@hamishwillee hamishwillee Mar 10, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

@sfuhrer Sadly yes. Once something has a fixed meaning then it is very hard to change, because if you do then any GCS that updates to the new value will not understand the messages from any vehicle that has not. We know that the update cycle for vehicles is of the order of 5 - 10 years across various flight stacks, so that's not viable.

That is why I am so adamant that we don't assign any meaning to reserved values in the other discussion #1808 (comment) - because if we do then we can't rename them without some degree of management, or do much else with them.

What we could do is create the new value as well and mark the old value as deprecated. I'm in two minds about that approach. On one hand it makes the pattern clear and means that "one day" you might be able to remove the inconsistent value. On the other hand the value is a bit ugly, and still has to exist until that "one day" comes.
I lean towards this approach, but I'd be interested in what both you and @DonLakeFlyer think.

Copy link
Contributor

@DonLakeFlyer DonLakeFlyer Mar 10, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

If the value remains the same for the new name. And the name MAV_TYPE_VTOL_DUOROTOR is always moved to the dustbin forever and never reused. Then there is no problem renaming. You can note the previous name in the text if you want. It will just cause a GCS to change names as well and recompile. This sort of thing happens all the time when new fields are added to a mavlink message. That causes the need for a code change and recompile as well. All of this continues to work in a forward compat without the code change as well.

Creating a new value for just a rename breaks forward compat without a GCS update.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Sounds OK to me, but will address in a separate PR because this does force a rebuilt for anyone using the item.

</entry>
<entry value="20" name="MAV_TYPE_VTOL_QUADROTOR">
<description>Quad-rotor VTOL using a V-shaped quad config in vertical operation. Tailsitter.</description>
<description>Quad-rotor Tailsitter VTOL using a V-shaped quad config in vertical operation. Note: should be named MAV_TYPE_VTOL_TAILSITTER_QUADROTOR.</description>
</entry>
<entry value="21" name="MAV_TYPE_VTOL_TILTROTOR">
<description>Tiltrotor VTOL</description>
<description>Tiltrotor VTOL. Fuselage and wings stay (nominally) horizontal in all flight phases. It able to tilt (some) rotors to provide thrust in cruise flight.</description>
</entry>
<!-- Entries up to 25 reserved for other VTOL airframes -->
<entry value="22" name="MAV_TYPE_VTOL_RESERVED2">
<description>VTOL reserved 2</description>
<entry value="22" name="MAV_TYPE_VTOL_FIXEDROTOR">
<description>VTOL with separate fixed rotors for hover and cruise flight. Fuselage and wings stay (nominally) horizontal in all flight phases.</description>
hamishwillee marked this conversation as resolved.
Show resolved Hide resolved
</entry>
<entry value="23" name="MAV_TYPE_VTOL_RESERVED3">
<description>VTOL reserved 3</description>
<entry value="23" name="MAV_TYPE_VTOL_TAILSITTER">
<description>Tailsitter VTOL. Fuselage and wings orientation changes depending on flight phase: vertical for hover, horizontal for cruise. Use more specific VTOL MAV_TYPE_VTOL_DUOROTOR or MAV_TYPE_VTOL_QUADROTOR if appropriate.</description>
</entry>
<!-- Entries 24/25 reserved for other VTOL airframe -->
<entry value="24" name="MAV_TYPE_VTOL_RESERVED4">
<description>VTOL reserved 4</description>
</entry>
Expand Down