cyclonedds icon indicating copy to clipboard operation
cyclonedds copied to clipboard

Python IDL conversion failing mid file

Open FirebarSim opened this issue 1 year ago • 9 comments

While attempting to convert the IDL for the OMG OARIS 2.0 standard https://www.omg.org/cgi-bin/doc?dtc/22-05-06.zip to a python module the IDL generation seems to fail partway through a file. Generation stops with no error raised by IDLC mid type definition on multiple files.

I do not know if this is me not understanding the use of IDLC or some other failure.

reproduction steps:

  1. Download OARIS DDS IDL
  2. Run idlc -l py iteratively through all files in the download
  3. Numerous files will not fully generate, e.g. _Track_Reporting.cache is an incomplete python file.

Hopefully there is just a step I have missed. Any help appreciated.

FirebarSim avatar Dec 12 '23 09:12 FirebarSim

I just tried Track_Reporting.idl with some unspecified but master-like version and it gives me a org/omg/c4i/Domain_Model/Sensor_Domain/Track_Reporting/_Track_Reporting.py that ends like:

@dataclass
@annotate.keylist(["subsystem_id"])
@annotate.final
@annotate.autoid("sequential")
class sensor_track_set_type(idl.IdlStruct, typename="org.omg.c4i.Domain_Model.Sensor_Domain.Track_Reporting.sensor_track_set_type"):
    element: 'org.omg.c4i.Domain_Model.Sensor_Domain.Track_Reporting.sensor_track_set_element_type'
    subsystem_id: 'org.omg.c4i.Domain_Model.Common_Types.subsystem_id_type'

I also don't get any *.cache files. It sounds to me like idlc crashed for you because of a bug that has been fixed since the version you are using.

Is there any chance you could try the latest version?

eboasson avatar Jan 12 '24 12:01 eboasson

I have now retried this with both the latest master version (of python module), 10.2.0, and the build from pip install cyclonedds. All have failed without generating anything except empty .cache files. All this was agains main release 10.4.0. I have tried master and no joy there either.

Currently running on a mostly blank windows 10 vm with pretty much nothing but cyclonedds, cmake, vsbuild tools, and vscode installed.

Does anyone know if there is a way to get a more verbose view of what is happening?

FirebarSim avatar Jan 16 '24 18:01 FirebarSim

Issue confirmed on another machine, steps to repeat:

  1. Download cyclonedds-0.10.4 and cyclonedds-python-0.10.2
  2. Install latest cmake
  3. Install latest python 3.12 to c:\python312, copy python.exe to python3.exe
  4. Install latest Visual Studio Build Tools
  5. Extract zip files to c:\temp\cyclonedds-0.10.4 and cyclonedds-python-0.10.2
  6. In c:\temp\cyclonedds-0.10.4: mkdir build
  7. In c:\temp\cyclonedds-0.10.4\build cmake -G "Visual Studio 17 2022" -DCMAKE_INSTALL_PREFIX=C:\cyclonedds ..
  8. In c:\temp\cyclonedds-0.10.4\build cmake --build .
  9. In c:\temp\cyclonedds-0.10.4\build cmake --build . --target install
  10. Add CYCLONEDDS_HOME to System Environment Variables and C:\cyclonedds\bin to System Path
  11. In c:\temp\cyclonedds-python-0.10.2 pip install .
  12. Download OARIS IDL Archive dtc-22-05-06 from OMG
  13. Extract IDL to c:\temp\idl
  14. In c:\temp\idl idlc -l py Track_Reporting.idl

This results in a number of empty .cache files and a folder structure containing it.

FirebarSim avatar Jan 17 '24 14:01 FirebarSim

I'll try it on Windows, it is definitely possible it is different on Windows than on macOS. If I don't do this soon, please bump the issue again ...

eboasson avatar Jan 26 '24 13:01 eboasson

I span up an Ubuntu VM and idlc worked flawlessly on the OARIS IDL, I assume there must be some flaw introduced by Windows.

There do look to be a couple of parsing bugs in idlc related to duplicate Enum value definitions when two different enums have a key with the same name e.g. Enum1.Estimated and Enum2.Estimated though.

FirebarSim avatar Jan 29 '24 14:01 FirebarSim

@FirebarSim thanks for checking!

It should work on Windows, too, so the bug still needs to be fixed. I'm happy that you have been able to work around it.

There do look to be a couple of parsing bugs in idlc related to duplicate Enum value definitions when two different enums have a key with the same name e.g. Enum1.Estimated and Enum2.Estimated though.

I'm puzzled by this. I just re-downloaded the file you linked and did:

# for x in *.idl ; do echo $x ; .../idlc -l .../_idlpy.cpython-311-darwin.so -x final $x ; done
# for x in *.idl ; do echo $x ; .../idlc -x final $x ; done
# clang -Wall -I .../include -c *.c

and I am not getting any errors, warnings (thanks to -x final) or seeing anything suspicious in a visual inspection of the output. I also can't find an Estimated anywhere in the IDL. Perhaps it is a different file.

In any case, if it is effectively this:

enum Enum1 { Estimated };
enum Enum2 { Estimated };

That's simply disallowed by the IDL spec. From the IDL 4.2 spec section 7.5.2 (Scoping Rules and Name Resolution):

The following kinds of definitions form scopes: modules, structures, unions, maps, interfaces, value types, operations, exceptions, event types, components, homes. [...]

so not for enums. It also includes an example in that section (the comments are part of the spec):

interface A {
  enum E { E1, E2, E3 }; // line 1
  enum BadE { E3, E4, E5 }; // Error: E3 is already introduced into the A scope in line 1 above
};

If it is something else, then perhaps you can share it so we can figure out what's going on.

eboasson avatar Jan 30 '24 10:01 eboasson

That’s correct, it’s a different file, specifically the OMG C2INAV standard and it’s exactly the kind of case you link.

It’s pretty funny that OMG aren’t following their own IDL spec.

FirebarSim avatar Jan 30 '24 10:01 FirebarSim

That’s correct, it’s a different file, specifically the OMG C2INAV standard and it’s exactly the kind of case you link.

🙄

It’s pretty funny that OMG aren’t following their own IDL spec.

I guess this adds another item to the list or reasons I don't consider the OMG a respectable standards body ...

I don't know yet what the correct solution is in this case.

eboasson avatar Jan 30 '24 11:01 eboasson

The correct solution is the one that someone has already taken on the OMG issues list I guess, reporting the issue as a required change.

For me I guess it’s renaming the enum to Estimated2 and changing it back after compilation!

FirebarSim avatar Jan 30 '24 11:01 FirebarSim