search

micro-ROS / micro_ros_diagnostics

Diagnostics framework for micro-ROS
Apache License 2.0
8 stars 4 forks source link

Working without strings #5

Closed mrbhjv closed 3 years ago

mrbhjv commented 3 years ago

Hi,

I'm quite glad to see there's a package like this for micro-ros, however, as you know, while working on embedded, one thing that you usually want to avoid is to have strings around at all. In our case, we are working with C++ on embedded and being very very cautious on not having any dynamic allocation or strings at all in our systems.

This said, wouldn't it be possible for the messages to use just codes, instead of strings? Then make a translation of that system on the higher-level system. I know that as long as the length of the string is less than 15 chars, the compiler will actually be able to optimize them, but I don't want to have to walk with that burden.

To summarize, we're trying to avoid any heap allocation and malloc will kind of mess with it.

Any suggestion?

norro commented 3 years ago

That's a reasonable request and I like your idea.

Currently we have five string fields in MicroROSDiagnosticStatus.msg, which need to be replaced:

  1. string name - A description of the test/component reporting.
  2. string message - A description of the status.
  3. string hardware_id - A hardware unique string.
  4. string key, string value - The actual key value pair

I can see the hardware_id potentially even set by the agent upon arrival, depending on the source of the message. For name, message, key, and value, the agent could hold a map, which translates codes to according Strings and KeyValues for vanilla diagnostic_msgs. This map could then also serve as an overview of valid/known codes for the respective microcontroller.

Does that sound reasonable to you?

mrbhjv commented 3 years ago

That sounds like a great idea! 👍

Regarding the agent setting up the hardware_id upon arrival, I don't think it's such a good idea, rather use a map as well on the agent side (or give the option). In our case, we have a subset of systems that run on different peripherals, like CAN, SPI, etc, and then they connect to a board that has ethernet, running the micro-ros client. This board acts as manager and needs to report the information of all these other hardware devices. Therefore, there will be multiple devices coming through the same channel.

pablogs9 commented 3 years ago

Usually, when dealing with strings in micro-ROS we don't use dynamically allocated string since our typesupport for XRCE-DDS does not need them.

For example, one of the approaches for this non-dynamic string handling can be seen here. Usually, throughout the micro-ROS stack, we try to avoid dynamic allocations as much as possible.

Does this make sense for this problem?

Regarding the agent-side handling, it should be possible. Maybe it also could be an external ROS 2 node that acts as a translator between micro-ROS status codes and the mapped complete message.

jamoralp commented 3 years ago

@pablogs9 I agree with you. We could think about a custom message type which is published on a specific topic, and then the micro-ROS Agent would subscribe to that topic and convert the defined error messages into a comprehensive enough text message.

norro commented 3 years ago

@jamoralp The diagnostic status message type was already custom to avoid using of arrays. In PR https://github.com/micro-ROS/micro_ros_diagnostics/pull/7 it is now also completely avoiding strings, requiring a bridge node to translate the custom micro-ROS diagnostic messages into vanilla diagnostic messages. See architecture diagram in branch feature/un-string-ify.

@mrbhjv Can you check if PR https://github.com/micro-ROS/micro_ros_diagnostics/pull/7 meets your expectations?

mrbhjv commented 3 years ago

I'm sorry for the delay, I gave it a small skim yesterday. We'll look at it now