Publish analog pins

[travis-mendoza]

This PR extends the turtlebot3_node to publish the raw values measured by the OpenCR pins A0-A5, read from the control table addresses defined in the OpenCR repo PR#342.

[gemini-code-assist]

Hello @travis-mendoza, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

Summary of Changes

This pull request introduces the capability to publish raw analog pin values (A0-A5) from the OpenCR controller on the Turtlebot3. It achieves this by adding a new AnalogPins sensor class, defining the control table entries for the analog pins, and integrating the new sensor into the Turtlebot3 node. The PR also updates the CMakeLists.txt to include the new source file.

Highlights

• New Sensor: Introduces a new AnalogPins sensor class to read and publish analog pin values.
• Control Table: Adds definitions for the analog pins (A0-A5) in the control table.
• Integration: Integrates the new AnalogPins sensor into the Turtlebot3 node, including initialization and publishing.
• Dynamixel SDK Wrapper: Initializes a separate read memory range for the analog pins in the Dynamixel SDK Wrapper.

Changelog

Click here to see the changelog

• turtlebot3_node/CMakeLists.txt
  • Added src/sensors/analog_pins.cpp to the list of source files to be compiled into the turtlebot3_node_lib library.
• turtlebot3_node/include/turtlebot3_node/control_table.hpp
  • Added ControlItem definitions for analog pins A0-A5, including their addresses, data types, and access permissions.
• turtlebot3_node/include/turtlebot3_node/sensors/analog_pins.hpp
  • Created a new header file for the AnalogPins class, defining its interface and dependencies, including the publisher for std_msgs::msg::UInt16MultiArray.
• turtlebot3_node/include/turtlebot3_node/turtlebot3.hpp
  • Included the turtlebot3_node/sensors/analog_pins.hpp header file.
• turtlebot3_node/src/sensors/analog_pins.cpp
  • Implemented the AnalogPins class, including the constructor to initialize the publisher and the publish method to read analog pin values from the OpenCR and publish them as a std_msgs::msg::UInt16MultiArray message.
• turtlebot3_node/src/turtlebot3.cpp
  • Removed the declaration of the namespace parameter.
  • Modified init_dynamixel_sdk_wrapper to initialize a separate read memory range for the analog pins.
  • Modified add_sensors to instantiate and add the AnalogPins sensor to the list of sensors.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in issue comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist is currently in preview and may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments to provide feedback.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

---

From pins of analog,
A reading we now can log,
Robot's senses grow.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[gemini-code-assist]

Code Review

This PR introduces the functionality to publish raw analog pin values from the OpenCR controller. The implementation appears well-structured and integrates smoothly with the existing codebase. However, there are a few areas that could benefit from refinement to enhance robustness and clarity.

Summary of Findings

• Error Handling in AnalogPins::publish: The current error handling in AnalogPins::publish catches exceptions but provides limited information about the specific error. Enhancing the error messages with more context would aid in debugging.
• Redundancy in Data Retrieval: The code retrieves data from the Dynamixel SDK wrapper for each analog pin individually. This could be optimized by reading all pin values in a single operation, if the underlying SDK supports it, to improve efficiency.
• Clarity of Variable Names: While the code is generally readable, using more descriptive variable names, especially for the analog_msg->layout.dim elements, could improve understanding.

Merge Readiness

The pull request is almost ready for merging. Addressing the error handling and potential data retrieval optimization would significantly improve the code's reliability and performance. I am unable to directly approve this pull request, and recommend that others review and approve this code before merging. I recommend that the author address the comments before merging.

[gemini-code-assist]

It might be beneficial to add a check to ensure that the analog_a5.addr is actually greater than analog_a0.addr to prevent potential issues if the control table is not defined as expected. This could be done with an RCLCPP_ERROR and a return if the condition is not met.

Suggested change

	// Add a separate initialization for the analog pins range
	dxl_sdk_wrapper_->init_read_memory(
	extern_control_table.analog_a0.addr,
	(extern_control_table.analog_a5.addr - extern_control_table.analog_a0.addr) +
	extern_control_table.analog_a5.length
	);
	if (extern_control_table.analog_a5.addr <= extern_control_table.analog_a0.addr) {
	RCLCPP_ERROR(this->get_logger(), "Invalid analog pin addresses in control table");
	return;
	}
	dxl_sdk_wrapper_->init_read_memory(
	extern_control_table.analog_a0.addr,
	(extern_control_table.analog_a5.addr - extern_control_table.analog_a0.addr) +
	extern_control_table.analog_a5.length
	);

[GyuH13]

Hello, @travis-mendoza
Thanks for your contribution. We'll review and discuss it, and reflect the changes or update the e-Manual accordingly. Thanks again.