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

Initial porting for mbed #6

Merged
merged 2 commits into from Oct 16, 2021

Conversation

smoriemb
Copy link

As I've reported below, I have some trivial problems to compile your mros2 for Mbed environment.
https://www.slideshare.net/smorita1/mros2-on-mbed

We know to implement an OS abstraction layer for the potability. But, it might be beneficial to port it , without such a layer, to another OS directly, as a first step.

smorita_emb added 2 commits October 10, 2021 15:05
- renamed string.h to cdr_string.h to avoid a link search path problem, which is caused by the same issue below.
  ARMmbed/mbed-cli#917
- added "return true;" at the tail of serializeIntoUdcrBuffer() to avoid a warning, which is treated as an error by Mbed online compiler (Arm Compiler).
@takasehideki
Copy link
Member

@smoriemb thank you so much for your contribution!!

I have one question to your commit, because I'm not so familiar with micro-CDR.
Why did you modify #include <ucdr/types/string.h> to #include <ucdr/types/cdr_string.h>?
Is it generally better to use cdr_string.h to handle String message for ROS2/DDS communication?
https://github.com/mROS-base/embeddedRTPS/pull/6/files#diff-5d1607a038f495dff32e5f4cad1145f56db8241cf21e4ab440507b1082a597b0

@smoriemb
Copy link
Author

Why did you modify #include <ucdr/types/string.h> to #include <ucdr/types/cdr_string.h>?
Is it generally better to use cdr_string.h to handle String message for ROS2/DDS communication?

It is not common, I think. This modification is just a workaround for the Web compiler.
As I said in the commit message, the Web compiler has some problem in the header search path.
It seems to confuse the "string.h" above with "string.h" of the standard library.
ARMmbed/mbed-cli#917

I found this problem only in the Web compiler, and there is no problem with CLI2(GCC).
So, if we finally decide to give up the support for the Web compiler, this modification will no longer be needed.

@takasehideki
Copy link
Member

Hmmm, I'm thinking deeply. Renaming third-party files just for the Mbed web compiler doesn't seem reasonable,,,

@takasehideki
Copy link
Member

I found Micro-CDR/include/ucdr/types/string.h is no longer included the latest version of Micro-CDR officially published by eProsima.
https://github.com/eProsima/Micro-CDR/tree/master/include/ucdr

And also, header files seemed to be merged to one file.
eProsima/Micro-CDR@1f04882#diff-9afa999489aaed4e8a94abb6e5de2470cdd2459e0f3f36b1e43bf10b5bc4a5c3

I think that our possible options are 1) to allow file renaming and 2) to follow the update to Micro-CDR.
I sent questions as an issue to embeddedRTPS developer team.
embedded-software-laboratory#7

@takasehideki
Copy link
Member

we decided to merge this PR as it is.

@takasehideki takasehideki merged commit 863aff7 into mROS-base:mros2 Oct 16, 2021
takasehideki added a commit to mROS-base/mros2 that referenced this pull request Oct 16, 2021
takasehideki pushed a commit that referenced this pull request Dec 23, 2021
* minor changes required for compilation with Tasking Compiler for Infineon Aurix
* fixing errors occurring when activating verbose outputs
* clang-format

Co-authored-by: Alexandru Kampmann <kampmann@embedded.rwth-aachen.de>
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.

None yet

2 participants