ArduPilot / pymavlink

python MAVLink interface and utilities
Other
498 stars 597 forks source link

C++11 generation fails if an enum value is a substring of the enum name #887

Open viktornordling opened 11 months ago

viktornordling commented 11 months ago

If you have this enum:

    <enum name="SOME_ENUM_NAME">
      <entry value="0" name="SOME_ENUM_NAME_VALUE_1"/>
    </enum>

And run:

mavgen.py --lang=C++11 --wire-protocol=2.0 test.xml

Things work as expected. However, if you have:

    <enum name="SOME_ENUM_NAME">
      <entry value="0" name="SOME_ENUM"/>
    </enum>

The result is:

Generating C++ implementation in directory /var/folders/v1/_6c7qt5d2mz4hsg5zm7l0n400000gp/T/tmp.hK32dYxj/test
Traceback (most recent call last):
  File "/nix/store/qm3y7k1a4w7d53r758m9hr50higs685k-python3.10-pymavlink-2.4.35/bin/.mavgen.py-wrapped", line 32, in <module>
    mavgen.mavgen(args, args.definitions)
  File "/nix/store/qm3y7k1a4w7d53r758m9hr50higs685k-python3.10-pymavlink-2.4.35/lib/python3.10/site-packages/pymavlink/generator/mavgen.py", line 283, in mavgen
    mavgen_cpp11.generate(opts.output, xml)
  File "/nix/store/qm3y7k1a4w7d53r758m9hr50higs685k-python3.10-pymavlink-2.4.35/lib/python3.10/site-packages/pymavlink/generator/mavgen_cpp11.py", line 456, in generate
    generate_one(basename, xml)
  File "/nix/store/qm3y7k1a4w7d53r758m9hr50higs685k-python3.10-pymavlink-2.4.35/lib/python3.10/site-packages/pymavlink/generator/mavgen_cpp11.py", line 429, in generate_one
    f.name_trim = enum_remove_prefix(e.name, f.name)
  File "/nix/store/qm3y7k1a4w7d53r758m9hr50higs685k-python3.10-pymavlink-2.4.35/lib/python3.10/site-packages/pymavlink/generator/mavgen_cpp11.py", line 302, in enum_remove_prefix
    if pl[i] == sl[0]:
IndexError: list index out of range

Here's a table of what works and what doesn't:

enum_value | result
===================
S => works
SO => works
SOM => works
SOME => ERROR
SOME_ => works
SOME_E => works
SOME_EN => works
SOME_ENU => works
SOME_ENUM => ERROR
SOME_ENUM_ => works
SOME_ENUM_N => works
SOME_ENUM_NA => works
SOME_ENUM_NAM => works
SOME_ENUM_NAME => ERROR
SOME_ENUM_NAME_ => works
SOME_ENUM_NAME_...(anything) => works

Hope this helps with debugging! :)

shancock884 commented 11 months ago

I had a super-quick look. I think the answer is to change line 306 of mavgen_cpp11.py (in function enum_remove_prefix) from:

if pl[i] == sl[0]:

to:

if len(sl) > 1 and pl[i] == sl[0]:

which protects against the abbreviated enum name ending up empty.

@viktornordling - feel free to try the change on your local version if you like to try it out, and when I get a mo, I can PR it.

viktornordling commented 11 months ago

Thanks @shancock884. I'll give it a spin, though I'd love to see a unit test as well! :)

shancock884 commented 11 months ago

The test I did before and after the change was to edit my local xml and temporarily change 'FIRMWARE_VERSION_TYPE_OFFICIAL' to 'FIRMWARE_VERSION', and then run the generator command.

I am not yet familiar with the repo automated tests for cpp11, to know if this could be included there.

On Sun, 17 Dec 2023, 06:55 viktornordling, @.***> wrote:

Thanks @shancock884 https://github.com/shancock884. I'll give it a spin, though I'd love to see a unit test as well! :)

— Reply to this email directly, view it on GitHub https://github.com/ArduPilot/pymavlink/issues/887#issuecomment-1859055510, or unsubscribe https://github.com/notifications/unsubscribe-auth/BBRZQQQWIO7X6OA6WOUR6M3YJ2JMTAVCNFSM6AAAAABANT4APSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQNJZGA2TKNJRGA . You are receiving this because you were mentioned.Message ID: @.***>

shancock884 commented 11 months ago

Thinking about it, I think this will still not cover the case where the enum name has a trailing "_".