Skip to content

Conversation

@Myzhar
Copy link
Member

@Myzhar Myzhar commented Aug 6, 2018

No description provided.

Myzhar added 5 commits August 3, 2018 15:35
 * publishing on its own thread
 * PCL dependency removed
 * Elaboration time reduced from 70msec to about 10 msec
Added sl_tools to easily measure time
@Myzhar Myzhar requested review from adujardin and nesnes August 6, 2018 09:30
Copy link
Member

@adujardin adujardin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me (although the complete code reformatting didn't make that particularly easy ;) )
Check out the comments, there are a few things to correct.
Also, you should check a potential issue I found when messing around with unsub/sub to DepthCloud or PointCloud2 in Rviz the cloud would freeze or disappear for some time (like 5-10 sec with everything but the cloud)

CMakeLists.txt Outdated
set(TOOLS_SRC ${CMAKE_CURRENT_SOURCE_DIR}/src/tools/src/sl_tools.cpp)
set(TOOLS_SRC
${CMAKE_CURRENT_SOURCE_DIR}/src/tools/src/sl_tools.cpp
${CMAKE_CURRENT_SOURCE_DIR}/src/tools/src/stopwatch.cpp )
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you forgot to push this file. However, arguably I'd put it in sl_tools.cpp, also do we actually need a custom stopwatch? There is already a macro in the ZED SDK (INIT_TIMER, STOP_TIMER(str))

<arg name="serial_number" default="0" />
<arg name="verbose" default="true" />
<arg name="resolution" default="2" /> <!--0=RESOLUTION_HD2K, 1=RESOLUTION_HD1080, 2=RESOLUTION_HD720, 3=RESOLUTION_VGA -->
<arg name="frame_rate" default="30" />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think 30 fps is a better default value, 60 is just too much data to process

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants