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.

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

Python pub to C sub fails when using large/complex IDL #190

Open
smnrgrs opened this issue Apr 17, 2023 · 6 comments
Open

Python pub to C sub fails when using large/complex IDL #190

smnrgrs opened this issue Apr 17, 2023 · 6 comments

Comments

@smnrgrs
Copy link

smnrgrs commented Apr 17, 2023

I've been struggling for a while with Python/C interoperability when using a complex topic (simpler ones work). On one Ubuntu 20.04 PC, Python to C interoperability fails (but succeeds if I comment out some of the topic fields). A clue is an OpenDDS subscriber says 'ERROR: Sedp::TypeLookupRequestReader::data_received_i - failed to take type lookup request'

I don't really know how to debug further so I've forked good old i11eperf and made branch 'complex-union' to demonstrate the problem: git clone https://github.com/smnrgrs/i11eperf.git and switch to branch 'complex-union'.

I have extended the 'ou' topic, and added a python publisher in the Python folder.

I see the following:
Python pub -> osub FAIL
Python pub -> asub FAIL
apub -> osub OK
Python pub -> Python sub OK

If the case TDTypeBd is removed from the IDL union at line 260, then interoperability works. Any suggestions would be gratefully received.

@eboasson
Copy link
Contributor

I think #191 does the trick, but I have only checked Python sub + apub and Python pub + asub.

@smnrgrs
Copy link
Author

smnrgrs commented Apr 20, 2023

Thanks for looking at it so quickly, initial tests look promising, I'll update with something more definitive as soon as I can.

@smnrgrs
Copy link
Author

smnrgrs commented Apr 24, 2023

Your change has certainly improved Python pub -> osub. There's possibly still some kind of issue with multi-line // comments in enums, I haven't quite figured it out yet.

@smnrgrs
Copy link
Author

smnrgrs commented May 6, 2023

The changes in #191 certainly help @eboasson, but there are more problems. I've committed an update to the idl in https://github.com/smnrgrs/i11eperf.git which has more data in the union (and is closer to my full topic). With #191 in use the same test (python pub to asub/osub) fails, but works when an arbitrary change is made to the name of a data item (https://github.com/smnrgrs/i11eperf/blob/complex-union/idl/i11eperf.idl.works)

@eboasson
Copy link
Contributor

eboasson commented May 8, 2023

Most curious!

Looking at python pub and asub, the funny thing is that this time the type identifiers (and hence type objects) are identical. Now it is the deserialisation of the data published by the python publisher that fails if the field is named dPValux and succeeds when it is named dPValue.

It does seem to make a difference if I change pub.py to use the correct field name. But surely it should never generate invalid CDR 🤔

(That's with https://github.com/eboasson/cyclonedds/tree/dynsub-typeobj-printer — I didn't check yet whether I might've added some change besides extending dynsub. I just wanted a quick look, so I skipped the homework!)

@smnrgrs
Copy link
Author

smnrgrs commented May 10, 2023

Well I'm glad you can reproduce the failure!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants