Skip to content

Honda ssc debug#139

Merged
cullenstoneAS merged 14 commits intoros1_masterfrom
honda_ssc_debug
Jun 7, 2023
Merged

Honda ssc debug#139
cullenstoneAS merged 14 commits intoros1_masterfrom
honda_ssc_debug

Conversation

@cullenstoneAS
Copy link
Copy Markdown
Contributor

add missing messages to support Honda SSC
add support for global_rpt_2 for /pacmod/enabled topic

Comment thread src/pacmod3_ros_msg_handler.cpp Outdated
Comment thread src/pacmod3_nodelet.cpp Outdated
Comment thread include/pacmod3/pacmod3_nodelet.h Outdated
@@ -141,7 +141,9 @@ class Pacmod3Nl : public nodelet::Nodelet
ros::Publisher all_system_statuses_pub;
ros::Publisher global_rpt_pub;
ros::Publisher global_rpt_2_pub;
Copy link
Copy Markdown

@jilinzhouas jilinzhouas Jun 7, 2023

Choose a reason for hiding this comment

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

All those publisher variables stored into pub_tx_list with std::move should be deleted here.

Comment thread src/pacmod3_nodelet.cpp Outdated
@@ -62,7 +64,9 @@ void Pacmod3Nl::onInit()
vin_rpt_pub = nh_.advertise<pacmod3_msgs::VinRpt>("vin_rpt", 5);

pub_tx_list.emplace(GLOBAL_RPT_CANID, std::move(global_rpt_pub));
Copy link
Copy Markdown

@jilinzhouas jilinzhouas Jun 7, 2023

Choose a reason for hiding this comment

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

After std::move, these independent publish variables are no longer usable. How about create them as local variables in this function.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I agree we should improve this, but maybe create a separate issue since it's unrelated to the changes in this PR.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@icolwell-as sorry I didn't read your comment before I changed it, should I revert the changes for this PR?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

No worries! if the work is done, all the better, just didn't want to hold the PR on something unrelated.
All good! merge at will

@cullenstoneAS cullenstoneAS merged commit fe47ed8 into ros1_master Jun 7, 2023
@cullenstoneAS cullenstoneAS deleted the honda_ssc_debug branch June 7, 2023 20:54
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.

4 participants