ArduPilot / pymavlink

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

Generating Swift library fails with compile error #841

Open valeriyvan opened 1 year ago

valeriyvan commented 1 year ago
python3 -m pymavlink.tools.mavgen --lang=Swift --wire-protocol=2.0 --output=generated/swift/mavlink/v2.0 message_definitions/v1.0/all.xml
Validating message_definitions/v1.0/all.xml
Parsing message_definitions/v1.0/all.xml
Validating /Users/*****/DronePro/mavlink/message_definitions/v1.0/ardupilotmega.xml
Parsing /Users/*****/DronePro/mavlink/message_definitions/v1.0/ardupilotmega.xml
Validating /Users/*****/DronePro/mavlink/message_definitions/v1.0/ASLUAV.xml
Parsing /Users/*****/DronePro/mavlink/message_definitions/v1.0/ASLUAV.xml
Validating /Users/*****/DronePro/mavlink/message_definitions/v1.0/common.xml
Parsing /Users/*****/DronePro/mavlink/message_definitions/v1.0/common.xml
Validating /Users/*****/DronePro/mavlink/message_definitions/v1.0/development.xml
Parsing /Users/*****/DronePro/mavlink/message_definitions/v1.0/development.xml
Validating /Users/*****/DronePro/mavlink/message_definitions/v1.0/icarous.xml
Parsing /Users/*****/DronePro/mavlink/message_definitions/v1.0/icarous.xml
Validating /Users/*****/DronePro/mavlink/message_definitions/v1.0/minimal.xml
Parsing /Users/*****/DronePro/mavlink/message_definitions/v1.0/minimal.xml
Validating /Users/*****/DronePro/mavlink/message_definitions/v1.0/python_array_test.xml
Parsing /Users/*****/DronePro/mavlink/message_definitions/v1.0/python_array_test.xml
Validating /Users/*****/DronePro/mavlink/message_definitions/v1.0/standard.xml
Parsing /Users/*****/DronePro/mavlink/message_definitions/v1.0/standard.xml
Validating /Users/*****/DronePro/mavlink/message_definitions/v1.0/test.xml
Parsing /Users/*****/DronePro/mavlink/message_definitions/v1.0/test.xml
Validating /Users/*****/DronePro/mavlink/message_definitions/v1.0/ualberta.xml
Parsing /Users/*****/DronePro/mavlink/message_definitions/v1.0/ualberta.xml
Validating /Users/*****/DronePro/mavlink/message_definitions/v1.0/uAvionix.xml
Parsing /Users/*****/DronePro/mavlink/message_definitions/v1.0/uAvionix.xml
Validating /Users/*****/DronePro/mavlink/message_definitions/v1.0/storm32.xml
Parsing /Users/*****/DronePro/mavlink/message_definitions/v1.0/storm32.xml
Validating /Users/*****/DronePro/mavlink/message_definitions/v1.0/AVSSUAS.xml
Parsing /Users/*****/DronePro/mavlink/message_definitions/v1.0/AVSSUAS.xml
Validating /Users/*****/DronePro/mavlink/message_definitions/v1.0/cubepilot.xml
Parsing /Users/*****/DronePro/mavlink/message_definitions/v1.0/cubepilot.xml
Validating /Users/*****/DronePro/mavlink/message_definitions/v1.0/csAirLink.xml
Parsing /Users/*****/DronePro/mavlink/message_definitions/v1.0/csAirLink.xml
Merged enum MAV_CMD
Merged enum MAV_CMD
Merged enum MAV_CMD
Merged enum MAV_CMD
Merged enum MAV_CMD
Merged enum MAV_CMD
Found 355 MAVLink message types in 16 XML files
Traceback (most recent call last):
  File "<frozen runpy>", line 198, in _run_module_as_main
  File "<frozen runpy>", line 88, in _run_code
  File "/Users/*****/DronePro/mavlink/pymavlink/tools/mavgen.py", line 31, in <module>
    mavgen.mavgen(args, args.definitions)
  File "/Users/*****/DronePro/mavlink/pymavlink/generator/mavgen.py", line 291, in mavgen
    mavgen_swift.generate(opts.output, xml)
  File "/Users/*****/DronePro/mavlink/pymavlink/generator/mavgen_swift.py", line 376, in generate
    generate_messages_type_info(msgs, enums)
  File "/Users/*****/DronePro/mavlink/pymavlink/generator/mavgen_swift.py", line 347, in generate_messages_type_info
    field.initial_value = "try data." + swift_types[field.type][2] % field.wire_offset
                                        ~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~
TypeError: not enough arguments for format string

This is crossposting of original issue https://github.com/mavlink/mavlink/issues/2018.

peterbarker commented 1 year ago
> python3 -m pymavlink.tools.mavgen --lang=Swift --wire-protocol=2.0 --output=generated/swift/mavlink/v2.0 

So that's not a correct invocation; this is more like it:

python3 tools/mavgen.py --lang=Swift --wire-protocol=2.0 --output=/tmp/xyzzy $HOME/rc/ardupilot/modules/mavlink/message_definitions/v1.0/all.xml

... but that must have been a copy/paste goof.

So yes, this is an out-and-out bug in the swift generation. all.xml contains test.xml which contains tests for each mavlink type. The swift generator is not coping with "char" as a type. It tries to treat all char thingies as arrays, and that fails in this case as there's no length associated with a single char.

So I don't know Swift at all. Couldn't write it, so rather harder to fix a Swift code generator :-)

What should a single char be mapped to in Swift?

This is the table:

swift_types = {'char' : ("String", '"\\0"', "string(at: %u, length: %u)", "set(%s, at: %u, length: %u)"),
               'uint8_t' : ("UInt8", 0, "number(at: %u)", "set(%s, at: %u)"),
               'int8_t' : ("Int8", 0, "number(at: %u)", "set(%s, at: %u)"),
               'uint16_t' : ("UInt16", 0, "number(at: %u)", "set(%s, at: %u)"),
               'int16_t' : ("Int16", 0, "number(at: %u)", "set(%s, at: %u)"),
               'uint32_t' : ("UInt32", 0, "number(at: %u)", "set(%s, at: %u)"),
               'int32_t' : ("Int32", 0, "number(at: %u)", "set(%s, at: %u)"),
               'uint64_t' : ("UInt64", 0, "number(at: %u)", "set(%s, at: %u)"),
               'int64_t' : ("Int64", 0, "number(at: %u)", "set(%s, at: %u)"),
               'float' : ("Float", 0, "number(at: %u)", "set(%s, at: %u)"),
               'double' : ("Double", 0, "number(at: %u)", "set(%s, at: %u)"),
               'uint8_t_mavlink_version' : ("UInt8", 0, "number(at: %u)", "set(%s, at: %u)")}

That second column would seem to be the underlying Swift type - do we need to choose one of those to shove a single char into, or is there a better option?

Amusingly, if I remove the field from test.xml then the Swift generator completes generation from all.xml , which seems to tell me that char is never used as a type in any message definition in the messages specified by all.xml.

Your work-arounds are to nuke the type entry from all.xml, nuke the test.xml include from all.xml, or choose a narrower dialect (e.g. ardupilotmega.xml) until we get this sorted out.

e.g.

diff --git a/message_definitions/v1.0/test.xml b/message_definitions/v1.0/test.xml
index c6802074..25c79c02 100644
--- a/message_definitions/v1.0/test.xml
+++ b/message_definitions/v1.0/test.xml
@@ -4,7 +4,6 @@
   <messages>
     <message id="17000" name="TEST_TYPES">
       <description>Test all field types</description>
-      <field type="char" name="c">char</field>
       <field type="char[10]" name="s">string</field>
       <field type="uint8_t" name="u8">uint8_t</field>
       <field type="uint16_t" name="u16">uint16_t</field>
peterbarker commented 1 year ago

... note that it should probably not be a Swift Character. I guess that comes down to intent of char in the mavlink protocol. Any C-ish implementation is going to assume it's equivalent to an int8_t.

Frankly, I think we should probably just remove char as a valid type in MAVLink :-) @hamishwillee

hamishwillee commented 1 year ago

Frankly, I think we should probably just remove char as a valid type in MAVLink :-)

Good luck with that :-)

auturgy commented 1 year ago

char is used in param_ext (specifically char[128] ). It's one of the reasons I think param_ext should be nuked.

hamishwillee commented 1 year ago

PARAM_EXT_VALUE meets a use case and is in use. If we have an alternative proposal for meeting the use case then we can move (tortuously slowly) towards an option for replacing it. Just like anything else.