search

ros-drivers / rosserial

A ROS client library for small, embedded devices, such as Arduino. See: http://wiki.ros.org/rosserial
517 stars 525 forks source link

rosserial_arduino ESP8266 crashes on serialization #441

Closed logos-electromechanical closed 5 years ago

logos-electromechanical commented 5 years ago

I'm using the version of the library installed via the Library Manager (0.7.9). My hardware is an Adafruit Featherwing HUZZAH. I'm using the ESP9266 Arduino Core v2.5.2 and Arduino IDE 1.8.10.

I'm getting fatal exceptions every time I try to serialize a message. I found the cause, and have two proposed solutions; I'd like to hear from the community which one I ought to make into a PR.

All (or almost all) of the defined messages use constant strings as part of the message contents. These strings are currently stored in SPI flash and memcpy'd into outgoing buffers during serialization. Since the SPI flash can only be read in 4-byte aligned chunks, this causes fatal exceptions on ESP8266 Arduino Core v2.5.2 and Arduino IDE 1.8.10.

I have two proposed solutions:

-- Store all of the relevant strings in RAM. This definitely works, and is easy to implement. This may deplete RAM on applications that have lots of different message types compiled in. It's also not particularly elegant.

-- Write a modified memcpy and call it as appropriate when the address of the string to be copied is located in SPI flash. There may be other places that I didn't find where this causes problems, so this has the risk of not entirely fixing the issue.

Thoughts?

romainreignier commented 5 years ago

Apart on ESP8266, we generally avoid creating anything on the heap. So to keep the library usable on small microcontroller, we have to keep the constant strings. Maybe using #ifdef to handle the specific case of the ESP could be enough?

Le 26 septembre 2019 23:21:38 GMT+02:00, Pierce Nichols notifications@github.com a écrit :

I'm using the version of the library installed via the Library Manager (0.7.9). My hardware is an Adafruit Featherwing HUZZAH. I'm using the ESP9266 Arduino Core v2.5.2 and Arduino IDE 1.8.10.

I'm getting fatal exceptions every time I try to serialize a message. I found the cause, and have two proposed solutions; I'd like to hear from the community which one I ought to make into a PR.

All (or almost all) of the defined messages use constant strings as part of the message contents. These strings are currently stored in SPI flash and memcpy'd into outgoing buffers during serialization. Since the SPI flash can only be read in 4-byte aligned chunks, this causes fatal exceptions on ESP8266 Arduino Core v2.5.2 and Arduino IDE 1.8.10.

I have two proposed solutions:

-- Store all of the relevant strings in RAM. This definitely works, and is easy to implement. This may deplete RAM on applications that have lots of different message types compiled in. It's also not particularly elegant.

-- Write a modified memcpy and call it as appropriate when the address of the string to be copied is located in SPI flash. There may be other places that I didn't find where this causes problems, so this has the risk of not entirely fixing the issue.

Thoughts?

-- You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub: https://github.com/ros-drivers/rosserial/issues/441

logos-electromechanical commented 5 years ago

Conditional compilation strikes me as a good variation of my first idea. It's not entirely clear how to insert it at this level of rosserial -- how is that part of the code even written out?

logos-electromechanical commented 5 years ago

Something I find confusing -- if I generate the library with make_libraries.py, all of the strings are RAM strings rather than Flash strings. Where are the PSTR macros in the released libraries added?

romainreignier commented 5 years ago

The library that you install through the Library Manager from the Arduino application comes from this repo: https://github.com/frankjoshua/rosserial_arduino_lib

That's why the code is a bit different.

logos-electromechanical commented 5 years ago

Oh, ok -- I filed this bug there as well and he told me to do whatever I thought best. I'll close this and add a fix over there. Thanks!