[Arduino] Add ArduinoTcpHardware to use Arduino Ethernet shield. #324
Conversation
I have modified the ESP8266 hardware class to be able to use the Arduino Ethernet Shield for an Arduino TCP connexion. I have added the `#define ROSSERIAL_ARDUINO_TCP` to enable the TCP mode. Maybe another string is more suitable? The class has the same name `ArduinoHardware` in order to simplify the `ros.h` file. Only the included header file change. I have added two simple exemples. Tested with Arduino Nano, UNO, MEGA, Leonardo, DUE and ESP8266 based Wemos D1 Mini. Note that with the Atmega328, the RAM is limited after the include of the Ethernet and rosserial libraries.
| if(tcp_.connected()){ | ||
| return tcp_.read(); | ||
| }else{ | ||
| tcp_.connect(server_, serverPort_); | ||
| } |
There was a problem hiding this comment.
I know you're just following what was here before, but given that the file is basically being rewritten, it would be great if you could follow ROS brace style: http://wiki.ros.org/CppStyleGuide#Formatting
The astyle tool can quickly update the file: http://wiki.ros.org/roslint#Tips
There was a problem hiding this comment.
Ok, no problem, I will update the style.
| #include <SPI.h> | ||
| #include <Ethernet.h> | ||
|
|
||
| #define ROSSERIAL_ARDUINO_TCP |
There was a problem hiding this comment.
I'm not over the moon about switching behaviour based on this define, though it does kind of follow with what we've done previously, particularly around Leonardo and selecting whether to use the UART or USB connection.
I suppose the only alternatives are supplying a ros.h with custom NodeHandle typedefs in each example/firmware, which doesn't really make sense (though it's what we do internally at Clearpath).
There was a problem hiding this comment.
I agree it is not the best thing. I thing it should be better to include a different header file directly in the sketch. But it means revise the architecture of rosserial_client.
I don't see how you manage the typedef from the firmware, do you have a simple example?
There was a problem hiding this comment.
In our product firmwares, we're using rosserial ports that are not public— for example, that talk to the rosserial_server UDP implementation. In upstream rosserial (here), the ros.h file is supplied with the generated ros_lib and therefore has to be generic, but our ros.h files are much more use-case specific since they're instead supplied with the firmware source tree.
Anyway, it's a bit academic. I think what you have is fine for now, but it would be great to figure out how to document better for future users some suggested design patterns.
There was a problem hiding this comment.
About documentation, I think I will edit the rosserial_arduino wiki page to list all the available switches (USBCON, HW_SERIAL...).
About changing the design pattern, it would be nice to give a more explicit API but I have no idea how to do it right now.
|
Does my change about the white spaces is OK? |
|
Yes, looks great! Thanks for the contribution. |
I have modified the ESP8266 hardware class to be able to use the Arduino Ethernet Shield for an Arduino TCP connexion.
I have added the
#define ROSSERIAL_ARDUINO_TCPto enable the TCP mode. Maybe another string is more suitable?The class has the same name
ArduinoHardwarein order to simplify theros.hfile. Only the included header file change.I have added two simple exemples.
Tested with Arduino Nano, UNO, MEGA, Leonardo, DUE and ESP8266 based Wemos D1 Mini.
Note that with the Atmega328, the RAM is limited after the include of the Ethernet and rosserial libraries.