-
-
Notifications
You must be signed in to change notification settings - Fork 600
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
PlotJuggler with Fast-CDR-2.x.x #932
PlotJuggler with Fast-CDR-2.x.x #932
Conversation
switch (cdr_version_) | ||
{ | ||
case CdrVersion::XCDRv2: | ||
break; | ||
default: | ||
current_encoding_ = EncodingAlgorithmFlag::PLAIN_CDR; | ||
align64_ = 8; | ||
break; | ||
} |
Check notice
Code scanning / CodeQL
No trivial switch statements Note library
} | ||
else | ||
{ | ||
extra_size = 4; // EMHEADER; |
Check notice
Code scanning / CodeQL
Commented-out code Note library
} | ||
else | ||
{ | ||
extra_size = 4; // EMHEADER; |
Check notice
Code scanning / CodeQL
Commented-out code Note library
if (CdrVersion::XCDRv2 == cdr_version_ && | ||
EncodingAlgorithmFlag::PL_CDR2 != current_encoding_) | ||
{ | ||
// Take into account the boolean is_present; |
Check notice
Code scanning / CodeQL
Commented-out code Note library
e5b3db7
to
4e78ee9
Compare
find_package(fastcdr QUIET) | ||
|
||
if(NOT fastcdr_FOUND ) | ||
# Override Fast-CDR option: compile as static lib | ||
SET(BUILD_SHARED_LIBS OFF CACHE BOOL "Create shared libraries by default") | ||
add_subdirectory(3rdparty/Fast-CDR) | ||
include_directories(3rdparty/Fast-CDR/include) | ||
endif() | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand. If we are not compiling the library in 3rd party, how this even work?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I moved it to the upper makefile:
plotjuggler_plugins/ParserROS/CMakeLists.txt
My guess is that for some reason cmake does not propagate fastcdr to ParserRos CMakeLists.
And because ROSparser needs 3rdparty lib to link it tris to find fastCdr AT ROSParser level and fails.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe I could just move the find haven't tried yet.
I can test, but I won't be able to do it before next week
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I believe you should move that code back to the other Cmake, thanks!!!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Moved back 3rdparty build where it was.
had to leave find(fastcdr QUIET) though otherwise build would fail with fastcdr installed on build machine i.e
this issue: #861
Also rebased on latest main which is why (I think) windows CI does not pass.
If this commit is applied then plotjuggler will be able to compile and run with Fast-CDR version 2.x.x from eProsima. It can compile from: - local thirdparty folder included within the repo - install of fastcdr within the machine (usually /usr/local) to do so: - Udpated files, API and namespace names - Udpate configure.ac to match newly updated version Done it because I need to use plotJuggler with latest lib from eProsima and I would like this to be available for everyone. Issue: [facontidavide#919]
Added missing files from Fastcdr Ci shall build. PlotJuggler build again with or without fastcdr package installed. Issue: [facontidavide#919]
Moved up findCdr or build default library so that cmake does not fail before building when fastCdr is installed on system. Added message so that user knows when fastCdr is found on its system. Issue: [facontidavide#919] Co-authored-by: manuelValch
Remain compatible to fastCdr version 1.1.x so that build with Ros is not broken Issue: [facontidavide#919] Co-authored-by: manuelValch
As I love those splash screen tried to make a meme for it. Do not hesitate to drop it if you don't want it. Updated main and ressource to find the file Co-authored-by: ManuelValch <[email protected]>
If this commit is applied then fastCDR library will be build in original CMakeLists.txt file i.e ./rosx_introspection. Done that as it is a change requested in pull request. Issue: [facontidavide#919] Co-authored-by: ManuelValch <[email protected]>
0bc4393
to
d5a262d
Compare
Resolve: #919 #861
Reworked previous MR:
Added message if FastCdr is found on local PC:
If not found:
Tests
Tried to load some csv under
datasamples/rosbag2_test/
:Also able to stream data from fastDDS using eProsima plugin:
I can not test on old ros version but I hope it should be fixed now.