Skip to content
This repository has been archived by the owner on Jul 26, 2024. It is now read-only.

Support humble #257

Merged
merged 6 commits into from
Nov 28, 2022
Merged

Support humble #257

merged 6 commits into from
Nov 28, 2022

Conversation

tonynajjar
Copy link

@tonynajjar tonynajjar commented Oct 28, 2022

Fixes

#256

Description of the changes:

  • Added defaults to declare_parameter as is required in Humble
  • Added angles to Cmakelist to fix fatal error: angles/angles.h: No such file or directory
  • Minor compiler warning fixes
  • Removed "required" parameter as I don't see it used anywhere

Required before submitting a Pull Request:

I tested changes on:

  • [Not planning to] Windows
  • Linux
  • [ Not needed] ROS1
  • ROS2

Comment on lines 80 to 83
// this->declare_parameter({depth_raw_topic + compressed_format});
// this->declare_parameter({depth_raw_topic + compressed_png_level});
// this->declare_parameter({depth_rect_topic + compressed_format});
// this->declare_parameter({depth_rect_topic + compressed_png_level});
Copy link
Author

Choose a reason for hiding this comment

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

I'm not really sure here, these were parameters with their name being the topic name itself? What is the use here? Where are these used?

Copy link
Member

Choose a reason for hiding this comment

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

Honestly, not sure either. I think this was done as an alternative to topic remapping, but likely should be removed.

Copy link
Member

@ooeygui ooeygui left a comment

Choose a reason for hiding this comment

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

I'm approving this change. Thank you for the contribution!

Comment on lines 80 to 83
// this->declare_parameter({depth_raw_topic + compressed_format});
// this->declare_parameter({depth_raw_topic + compressed_png_level});
// this->declare_parameter({depth_rect_topic + compressed_format});
// this->declare_parameter({depth_rect_topic + compressed_png_level});
Copy link
Member

Choose a reason for hiding this comment

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

Honestly, not sure either. I think this was done as an alternative to topic remapping, but likely should be removed.

@tonynajjar
Copy link
Author

Honestly, not sure either. I think this was done as an alternative to topic remapping, but likely should be removed.

thought so too, I'll remove then

@ooeygui
Copy link
Member

ooeygui commented Oct 28, 2022

Don't worry about CI - I'll have to switch it to humble when the Humble Windows release is completed.

@tonynajjar
Copy link
Author

@tonynajjar please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.

@microsoft-github-policy-service agree [company="{your company}"]

Options:

  • (default - no company specified) I have sole ownership of intellectual property rights to my Submissions and I am not making Submissions in the course of work for my employer.
  • (when company given) I am making Submissions in the course of work for my employer (or my employer has intellectual property rights in my Submissions by contract or applicable law). I have permission from my employer to make Submissions and enter into this Agreement on behalf of my employer. By signing below, the defined term “You” includes me and my employer.

Contributor License Agreement

@microsoft-github-policy-service agree company="Pixel Robotics"

@st99315
Copy link

st99315 commented Nov 3, 2022

Hi, Do you get the error following when execute the launch?

$ ros2 launch azure_kinect_ros_driver driver.launch.py

[INFO] [launch]: All log files can be found below /root/.ros/log/2022-11-03-07-52-45-234984-jun-15580
[INFO] [launch]: Default logging verbosity is set to INFO
Robot description xacro_file : /root/ws_moveit/install/azure_kinect_ros_driver/share/azure_kinect_ros_driver/urdf/azure_kinect.urdf.xacro
Robot description urdf_path : /root/ws_moveit/install/azure_kinect_ros_driver/share/azure_kinect_ros_driver/urdf/azure_kinect.urdf
[ERROR] [launch]: Caught exception in launch (see debug for traceback): package 'azure_kinect_ros_driver' found at '/root/ws_moveit/install/azure_kinect_ros_driver', but libexec directory '/root/ws_moveit/install/azure_kinect_ros_driver/lib/azure_kinect_ros_driver' does not exist

Finally, it can be executed after revised CMakeLists.txt

install(TARGETS ${PROJECT_NAME}_node
  RUNTIME DESTINATION lib/${PROJECT_NAME}/
)

@tonynajjar
Copy link
Author

tonynajjar commented Nov 3, 2022

Thanks for the feedback, I haven't tried launching it yet actually as I didn't have a camera. I'll try it out and update you, at the latest next week.

@tonynajjar
Copy link
Author

@st99315 I can't seem to reproduce your bug. Did you make sure to source setup.bash after building?

@tonynajjar
Copy link
Author

@ooeygui FYI I am still adding changes to this PR as I am testing

@tonynajjar
Copy link
Author

I'm done here 👍

@tonynajjar
Copy link
Author

Any estimate on when this can be merged? Are we talking days or weeks?

@ooeygui
Copy link
Member

ooeygui commented Nov 28, 2022

Running CI; although it seems to be targeting Foxy.

@ooeygui ooeygui merged commit 6ffb95a into microsoft:humble Nov 28, 2022
@ooeygui
Copy link
Member

ooeygui commented Nov 28, 2022

Pulled to bootstrap the humble builds. Thank you for your contribution!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants