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

Fix deserialization endianness #336

Merged
merged 3 commits into from
Jul 25, 2023
Merged

Fix deserialization endianness #336

merged 3 commits into from
Jul 25, 2023

Conversation

Acuadros95
Copy link
Contributor

@Acuadros95 Acuadros95 commented Jul 12, 2023

Integration tests running on eProsima/Micro-XRCE-DDS#162 ✔️

@Acuadros95 Acuadros95 requested a review from pablogs9 July 12, 2023 10:57
@richiprosima
Copy link
Contributor

Build status:

  • Linux Build Status
  • Windows Build Status

Signed-off-by: acuadros95 <[email protected]>
Signed-off-by: acuadros95 <[email protected]>
@richiprosima
Copy link
Contributor

Build status:

  • Linux Build Status
  • Windows Build Status

@ted-miller
Copy link

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
Copy link

@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
Copy link

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
Copy link
Contributor Author

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

@gavanderhoorn
Copy link

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. 🍺

@Acuadros95
Copy link
Contributor Author

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.

🍻 !

@Acuadros95 Acuadros95 merged commit 2cbbc4a into develop Jul 25, 2023
@Acuadros95 Acuadros95 deleted the fix/endianness branch July 25, 2023 08:06
@gavanderhoorn
Copy link

@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
Copy link
Member

pablogs9 commented Aug 2, 2023

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
Copy link

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 👍

@gavanderhoorn
Copy link

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 👍 🍔

srmainwaring pushed a commit to srmainwaring/Micro-XRCE-DDS-Agent that referenced this pull request Aug 19, 2023
* Fix deserialization endianness

Signed-off-by: acuadros95 <[email protected]>

* Update doxy

Signed-off-by: acuadros95 <[email protected]>

* Add XRCE Endianness enum

Signed-off-by: acuadros95 <[email protected]>

---------

Signed-off-by: acuadros95 <[email protected]>
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

Successfully merging this pull request may close these issues.

5 participants