ArduPilot / ardupilot

ArduPlane, ArduCopter, ArduRover, ArduSub source
http://ardupilot.org/
GNU General Public License v3.0
10.96k stars 17.48k forks source link

Flight mode naming scheme is inconsistent #27713

Open ES-Alexander opened 3 months ago

ES-Alexander commented 3 months ago

Feature request

Is your feature request related to a problem? Please describe. The names of flight modes do not seem to follow an obvious logic as to when or where abbreviations or underscore-separators are applied, and several names seem unnecessarily hard for a human to make sense of. As examples, ALT_HOLD, POSHOLD, and GUIDED_NOGPS are in ArduCopter, while ArduPlane has more readable names like FLY_BY_WIRE_A.

Describe the solution you'd like Names should be consistent, and ideally descriptive of the functionality. From ArduPlane's LOITER_ALT_QLAND it seems the names can be at least 16 characters long, so I would propose underscores between all words, with full words where possible, e.g.:

Describe alternatives you've considered Control station softwares may already be doing some mapping to human-readable names, in which case end users may not run into much confusion with this, but that still doesn't resolve the issue for developers, or curious users looking at the implementation.

Incidentally, it might also be worth including some form of structured comments for the names, to provide a combined human-readable name and description that can be automatically extracted for use in documentation and interfacing softwares.

Platform [ ] All [ ] AntennaTracker [x] Copter [ ] Plane [ ] Rover [x] Submarine

rmackay9 commented 3 months ago

You may be right but I think it is probably too late to change them considering the changes to ground stations, wiki pages and other dependent systems some of which we don't even know about

peterbarker commented 3 months ago

@ES-Alexander you're suggesting we change the tokens throughout the code to be more English-like, I assume?

It wouldn't be an accident the authors chose the shorter strings - e.g. ALT_HOLD vs ALTITUDE_HOLD. They're really just tokens to help distinguish things, they're not really supported to be entirely descriptive of what the mode does. Trying to distinguish what LOITER does vs what POSHOLD does purely in the token name would be quite a challenge!

Your example of "ALT_HOLD" is interesting. Most of the Copter methods actually hold altitude. Making the name more descriptive - "ALTITUDE_HOLD" might actually be counterproductive as it may lead to people wondering how they combine LOITER and ALTITUDE_HOLD to keep their vehicle in the one spot in three dimensions, not realising LOITER does it all.

Note also that it is possible to programmatically extract the mode descriptions from the codebase. We do that for generating log messages, and enum_emit.py spits out a JSON file with this in it:

ARDUCOPTER_ENUMS = {

    "Mode::Number": {
        0: {
          'name': "STABILIZE",
          'description': "manual airframe angle with manual throttle",
        },
        1: {
          'name': "ACRO",
          'description': "manual body-frame angular rate with manual throttle",
        },
        2: {
          'name': "ALT_HOLD",
          'description': "manual airframe angle with automatic throttle",
        },
        3: {
          'name': "AUTO",
          'description': "fully automatic waypoint control using mission commands",
        },
        4: {
          'name': "GUIDED",
          'description': "fully automatic fly to coordinate or fly at velocity/direction using GCS immediate commands",
        },
        5: {
          'name': "LOITER",
          'description': "automatic horizontal acceleration with automatic throttle",
        },
        6: {
          'name': "RTL",
          'description': "automatic return to launching point",
        },
        7: {
          'name': "CIRCLE",
          'description': "automatic circular flight with automatic throttle",
        },
        9: {
          'name': "LAND",
          'description': "automatic landing with horizontal position control",
        },

Those mode descriptions are probably what you're really after in a user interface.

Williangalvani commented 3 months ago

@peterbarker could there be bad side-effects of doing the Sub change? ALT_HOLD to DEPTH_HOLD? I assume that mostly through the code and GCSs we are using the actual mode numbers instead of the string names.

Well I guess I can try and test if anything breaks.

peterbarker commented 3 months ago

@peterbarker could there be bad side-effects of doing the Sub change? ALT_HOLD to DEPTH_HOLD? I assume that mostly through the code and GCSs we are using the actual mode numbers instead of the string names.

Well I guess I can try and test if anything breaks.

The compiler won't care :-)

Just changing Mode::ALT_HOLD to Mode::DEPTH_HOLD won't even result in compiler output changes.

There's the question of whether this is all bike-shedding, 'though.