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

Noetic support #93

Merged
merged 5 commits into from
Jul 23, 2020
Merged

Noetic support #93

merged 5 commits into from
Jul 23, 2020

Conversation

chapulina
Copy link
Contributor

Changes since Melodic:

@chapulina chapulina requested a review from caguero as a code owner July 21, 2020 22:38
@chapulina
Copy link
Contributor Author

Commented out mav_msgs because that's not yet available on Noetic: ethz-asl/mav_comm#86

Copy link
Contributor

@j-rivero j-rivero left a comment

Choose a reason for hiding this comment

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

I found some code still enabled for Actuators:

ros_ign_bridge/test/publishers/ign_publisher.cpp:160:  auto actuators_pub = node.Advertise<ignition::msgs::Actuators>("actuators");
ros_ign_bridge/test/publishers/ign_publisher.cpp:161:  ignition::msgs::Actuators actuators_msg;
ros_ign_bridge/test/publishers/ign_publisher.cpp:162:  ros_ign_bridge::testing::createTestMsg(actuators_msg);
ros_ign_bridge/test/publishers/ign_publisher.cpp:214:    actuators_pub.Publish(actuators_msg);

ros_ign_bridge/test/launch/test_ros_subscriber.launch:33:              <!-- /actuators@mav_msgs/Actuators@ignition.msgs.Actuators -->
ros_ign_bridge/test/launch/test_ign_subscriber.launch:28:              /actuators@mav_msgs/Actuators@ignition.msgs.Actuators

ros_ign_bridge/test/test_utils.h:1425:  void createTestMsg(ignition::msgs::Actuators &_msg)
ros_ign_bridge/test/test_utils.h:1442:  void compareTestMsg(const ignition::msgs::Actuators &_msg)
ros_ign_bridge/test/test_utils.h:1444:    ignition::msgs::Actuators expected_msg;

Do we want to keep it?

Also found references to other ROS releases or Ignition versions from Blueprint. I made a branch chapulina/noetic_jrivero

.travis.yml Outdated Show resolved Hide resolved
Signed-off-by: Louise Poubel <louise@openrobotics.org>
chapulina and others added 4 commits July 22, 2020 17:17
Signed-off-by: Louise Poubel <louise@openrobotics.org>
Signed-off-by: Louise Poubel <louise@openrobotics.org>
@chapulina
Copy link
Contributor Author

Do we want to keep it?

I left the Ignition parts since they're already working. They're not very useful without the ROS side though. I guess at least we can keep it from rotting. If you think it's best to comment it out I can do that.

I made a branch

Thanks, merged it in.


I also rebased the noetic branch onto melodic after #72 and #96 - the noetic branch was just matching melodic before anyways, with no commits of its own.

@j-rivero
Copy link
Contributor

I left the Ignition parts since they're already working. They're not very useful without the ROS side though. I guess at least we can keep it from rotting. If you think it's best to comment it out I can do that.

Nothing important, I thought on someone listing gz topics and found the Actuator in there. A part from a bit of compile/runtime overhead without real use.

@chapulina chapulina merged commit 91265a7 into noetic Jul 23, 2020
@chapulina chapulina deleted the chapulina/noetic branch July 23, 2020 16:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏰 citadel Ignition Citadel ROS 1 ROS 1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants