search

eProsima / Micro-XRCE-DDS-Agent

Micro XRCE-DDS Agent respository. Looking for commercial support? Contact info@eprosima.com
Apache License 2.0
102 stars 72 forks source link

Fix deserialization endianness #336

Closed Acuadros95 closed 11 months ago

Acuadros95 commented 12 months ago

Integration tests running on https://github.com/eProsima/Micro-XRCE-DDS/pull/162 :heavy_check_mark:

richiprosima commented 12 months ago

Build status:

richiprosima commented 12 months ago

Build status:

ted-miller commented 12 months ago

Is this only expected to build in a Rolling environment? I'm attempting to build in Humble and getting an error regarding usage of rosidl_get_zero_initialized_type_hash. The only google reference I found to that function is in some Rolling document.

gavanderhoorn commented 12 months ago

@ted-miller: I believe the problem is on (y)our side. The default branch on micro-ROS-Agent is iron. You should be able to switch to the humble branch using git checkout humble.

ted-miller commented 12 months ago

Yep, I cloned the wrong branch.

My initial test looks promising. But I ran into additional issues in our code that is preventing me from fully testing. I'll continue debug next week.

Acuadros95 commented 11 months ago

@gavanderhoorn @ted-miller Any update? Can we merge this fix?

gavanderhoorn commented 11 months ago

Apologies for my/our delayed response @Acuadros95.

I haven't been able to test (any) more, but I believe @ted-miller reported he was able to get at least the initial Client<->Agent connection to succeed.

If these changes only affect the big-endian side, I'd suggest to merge, as it would seem it's currently in a non-working state, and this PR gets it at least closer to a working state, if not already a working state.

Thanks again for addressing this, and I owe you a beer -- if we ever run into you at a ROSCon or similar event. :beer:

Acuadros95 commented 11 months ago

If these changes only affect the big-endian side, I'd suggest to merge, as it would seem it's currently in a non-working state, and this PR gets it at least closer to a working state, if not already a working state.

The PR covers all the endianness communication issues, should be at a working state.

Thanks again for addressing this, and I owe you a beer -- if we ever run into you at a ROSCon or similar event.

:beers: !

gavanderhoorn commented 11 months ago

@Acuadros95: any tips on what would be the easiest way to build a docker image for the micro-ROS Agent which includes these changes?

I've been manually editing super build .cmake files so far.

Would that be the way to do it until a new release of Micro XRCE-DDS Agent is cut?

pablogs9 commented 11 months ago

Hello @gavanderhoorn, I guess that the last micro-ROS Agent has these changes. In any case I have retriggered the generation: https://github.com/micro-ROS/docker/actions/workflows/generate_agent_docker.yml

gavanderhoorn commented 11 months ago

Ah, ok. I got confused by the merge target for this PR (set to develop).

In any case I have retriggered the generation

thanks for that :+1:

gavanderhoorn commented 11 months ago

Just wanted to let you know I've been using the changes in this PR for some time now and it appears they've indeed fixed the issues I was having.

Thanks again for the fast turn-around @Acuadros95 and @pablogs9 👍 🍔