Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

initial implementation of Serial protocol #205

Merged
merged 10 commits into from
Oct 7, 2015
Merged

initial implementation of Serial protocol #205

merged 10 commits into from
Oct 7, 2015

Conversation

soundanalogous
Copy link
Member

Adds support for using Hardware and Software serial ports. See the protocol here.

All hardware ports except the default (Serial) port are supported. So you can use Serial1 on a Leonardo or Serial1, Serial2, or Serial3 on an Arduino Mega or Due.

Up to 4 SoftwareSerial instances can be used. Only one instance can read at a time however. Send the SERIAL_LISTEN message to set the current reading port.

Baud rates up to 115200 seem to work for reading using HW serial. Baud rates up to 57600 seem to work for reading using SW serial. When using a board that has additional HW serial ports (RX1, TX1, etc), using those ports will be more efficient than creating SoftwareSerial Instances.

Has been tested successfully with the following boards:

  • Uno (SoftwareSerial only)
  • Leonardo and Yun (SoftwareSerial or HardwareSerial via Serial1)
  • Mega (SoftwareSerial or HW Serial via Serial1, Serial2, Serial3)
  • Due (HW Serial via Serial1, Serial2, Serial3). SW serial not available.
  • Teensy++ 2.0 and Teensy 2.0 (HW Serial via Serial1)
  • Teensy 3.0 (HW Serial via Serial1, Serial2, Serial3)

This is a work in progress. Please test and report any bugs. This sketch can be used for testing. Even better would be to test against actual serial devices.

A client implementation and example are available here:

To do:

  • Move SERIAL_MESSAGE constant to Firmata.h
  • Add HW serial pin constants to Boards.h (temporary until added in Arduino)
  • Set Serial pin mode
  • Set HW serial RX pin mode to INPUT when SerialN.begin is called (fixes a bug with Due)
  • Report RX and TX (HW - RX1, TX2, RX2, TX2, RX3, TX3) pins in config query response
  • Consider moving Serial constants and functions to new serialUtils.h file

serialPort = NULL;
}
}
serialIndex = -1;
Copy link
Member Author

Choose a reason for hiding this comment

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

move this line outside of the directive block since it applies to HW serial as well

@rwaldron
Copy link
Contributor

Wow, will dig in this week :D

@soundanalogous
Copy link
Member Author

Turns out Arduino doesn't declare constants like RX, TX, RX1, TX1, etc so there is no easy way to provide this info in a configuration query response. Looks like I'll have to add constants to Boards.h per arch. For identifying pins that are SW serial capable is another matter since not all pins work on some boards. For now for HW Serial, I could map the UART pins in Boards.h by adding a IS_PIN_UART(p) macro to map which pins are HW serial pins. That would only indicate that the pins are Serial compatible and not which are RX or TX however. The better solution is to get Arduino to add constants for UART pins. The user does not need to query which HW serial pins are which unless they are using a non-labeled board so this may be a non issue, it would just be non-conventional to not include serial pins in the config query.

@rwaldron
Copy link
Contributor

rwaldron commented Aug 4, 2015

I had to remove all occurrences of
#if defined(ARDUINO_ARCH_AVR) to make this work. I did a search and found plenty of other uses of this constant, so I'm not sure what this means?

@soundanalogous
Copy link
Member Author

That is strange you got an error on that define. It is necessary because SoftwareSerial is only compatible with AVR micros. It will fail to compile if you include SoftwareSerial with a non-AVR board. What board are you using and which version of the Arduino IDE?

@soundanalogous
Copy link
Member Author

Can you also copy the Arduino log output from a version with those defines in place?

@rwaldron
Copy link
Contributor

rwaldron commented Aug 4, 2015

Sure thing.. I'm using an Arduino Uno.

Here's the output from the first issue I encounter:

StandardFirmata.ino: In function 'void checkSerial()':
StandardFirmata:218: error: 'SoftwareSerial' was not declared in this scope

So I replaced the ifdef from the top:

#if defined(ARDUINO_ARCH_AVR)
#include <SoftwareSerial.h>
#endif

With

#include <SoftwareSerial.h>

Error is gone, but there is no reply delivered to serialRead (testing with another Arduino doing a simple Serial write). So then I removed all occurrences of #if defined(ARDUINO_ARCH_AVR) (and closing #endif), now it all works correctly. I'm using 1.0.6 (with the correct version of the library built from this branch source), because Arduino 1.6.4 wouldn't load the library (tried both importing built zip and directly copying the dir, it just didn't want to work).

@rwaldron
Copy link
Contributor

rwaldron commented Aug 4, 2015

This is stuck:

(irrelevant to our goals)

@soundanalogous
Copy link
Member Author

As of Arduino 1.6.4 you should be able to clone Firmata directly into ~/Documents/Arduino/libraries/ rather than installing into the Arduino application directory.

continue;
}
// only the SoftwareSerial port that is "listening" can read data
if (portId > 7 && !((SoftwareSerial*)serialPort)->isListening()) {
Copy link
Member Author

Choose a reason for hiding this comment

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

@rwaldron this appears to be the source of the error. Lines 218 - 220 should also be wrapped by:

#if defined(ARDUINO_ARCH_AVR)
...
#endif

Without those defines if you tried to compile on an ARM board like a Due or a Teensy, compilation would fail. There could be an additional issue of ARDUINO_ARCH_AVR not being included in Arduino 1.0.6. I need to verify later when I get home.

@soundanalogous
Copy link
Member Author

@rwaldron with the latest commit, it should work now in both Arduino 1.0.6 and Arduino 1.6.5. Haven't tested with 1.6.4 to see if that has other issues. It also appears that the preprocessor is handled differently in Arduino 1.0.x vs 1.6.x. I had no errors in 1.6.5 (even though there was clearly an error in the code) while 1.0.6 reported the same error you encountered.

@rwaldron
Copy link
Contributor

rwaldron commented Aug 5, 2015

As of Arduino 1.6.4 you should be able to clone Firmata directly into ~/Documents/Arduino/libraries/ rather than installing into the Arduino application directory.

Didn't try that, so I will.

with the latest commit, it should work now in both Arduino 1.0.6 and Arduino 1.6.5. Haven't tested with 1.6.4 to see if that has other issues. It also appears that the preprocessor is handled differently in Arduino 1.0.x vs 1.6.x. I had no errors in 1.6.5 (even though there was clearly an error in the code) while 1.0.6 reported the same error you encountered.

I will confirm both and report back!

@soundanalogous
Copy link
Member Author

Actually it doesn't work in Arduino 1.0.6 (when I tested I had compiled from 1.6.5 after proving that compilation succeeded in 1.0.6). ARDUINO_ARCH_AVR was added as of Arduino 1.5.x in order to differentiate between AVR and ARM based architectures. I may be able to use the following instead in order to support both Arduino 1.0.6 and 1.5.x and higher:

#if !defined(ARDUINO_ARCH_SAM)

This will prevent official arduino ARM boards from trying to include SoftwareSerial (which would fail because the library is not in the compile path for non AVR boards).

@rwaldron
Copy link
Contributor

rwaldron commented Aug 5, 2015

Ok, I will focus on >=1.5.x for now

@rwaldron
Copy link
Contributor

rwaldron commented Aug 5, 2015

Testing with IDE 1.6.4...

  • Confirmed with Uno to Uno reading, one running the lastest version of this PR and the other with:
#include <SoftwareSerial.h>

SoftwareSerial s(10, 11); // RX, TX

void setup() {
  s.begin(9600);
}

void loop() {
  s.write(1);
  s.write(2);
  s.write(3);
  s.write(4);  
  delay(20);
}
  • Successfully read from a GPS device
    • Adafruit GPS Breakout V3
  • Successfully wrote to a servo controller
    • LynxMotion SSC-32U servo controller

@rwaldron
Copy link
Contributor

rwaldron commented Aug 5, 2015

Next thing I'm going to work on:

Poor performance of non-serial devices attached to the board

@soundanalogous
Copy link
Member Author

I'm considering a couple of changes:

  1. Add a 'format' parameter to SERIAL_CONFIG and move bytesToRead to SERIAL_READ.
  2. Add a readInterval parameter to SERIAL_READ to specify the interval (in milliseconds) at which the serial port is read. The biggest potential issue here is it would add at least 72 extra bytes.

These 2 changes would update the SERIAL_CONFIG and SERIAL_READ sections of the proposed Serial protocol as follows:

Serial Config

Configures the specified hardware or software serial port. The format options (bytes 6 and 7) are
not supported by all Serial ports. See the documentation for your platform (Arduino, etc) regarding
which format (or "config") options are available per serial port per board.

0  START_SYSEX      (0xF0)
1  SERIAL_DATA      (0x60)  // command byte
2  SERIAL_CONFIG    (0x10)  // OR with port (0x11 = SERIAL_CONFIG | HW_SERIAL1)
3  baud             (bits 0 - 6)
4  baud             (bits 7 - 13)
5  baud             (bits 14 - 20) // need to send 3 bytes for baud even if value is < 14 bits
6  format           (bits 0 - 6)
7  format           (bits 7 - 13)
                    Set to 0 to ignore (and use default 8N1), otherwise set data,
                    parity and stop bits as follows:
                    {13-9: reserved} +
                    {8-5: data bits, B1000 = 8, etc} +
                    {4-2: parity, B000 = None, B001 = Even, B010 = Odd, B011 = Mark, B100 = Space} +
                    {1-0: stop bits, B00 = 1, B01 = 1.5, B10 = 2}
                    // Restrictions will apply per capabilities of individual serial ports
8  rxPin            (0-127) [softserial only] // restrictions apply per board
9  txPin            (0-127) [softserial only] // restrictions apply per board
10 END_SYSEX        (0xF7)

Serial Read

Board -> Firmata client

Read contents of serial buffer and send to Firmata client (send with SERIAL_REPLY).

bytesToRead and readInterval only apply when SERIAL_READ_MODE is set to SERIAL_READ_CONTINUOUSLY

0  START_SYSEX        (0xF0)
1  SERIAL_DATA        (0x60)
2  SERIAL_READ        (0x30) // OR with port (0x31 = SERIAL_READ | HW_SERIAL1)
3  SERIAL_READ_MODE   (0x00) // 0x00 => read continuously, 0x01 => stop reading
4  bytesToRead        (bits 0 - 6)
5  bytesToRead        (bits 7 - 13)
                      if 0, read all available bytes per iteration of the main loop
                      else read the number of bytes specified per iteration
6  readInterval       (lsb) // milliseconds
7  readInterval       (msb)
                      if 0, read as frequently as possible, otherwise read at the specified interval
8  END_SYSEX          (0xF7)

@soundanalogous
Copy link
Member Author

An alternative approach to readInterval that would use less memory would be to set a single read interval that would be used for all serial ports. It would look something like this:

Serial Read Interval

[Optional] Set the readInterval in milliseconds at which the serial port is read. Default value is 0 which
will read as frequently as possible. Lower values such as 1 or 2ms may not be accurate depending on
how much time it takes the MCU to execute other tasks in the main loop.

0  START_SYSEX          (0xF0)
1  SERIAL_DATA          (0x60)
2  SERIAL_READ_INTERVAL (0x80)
3  readInterval         (lsb) // milliseconds
4  readInterval         (msb)
                        if 0, read as frequently as possible, otherwise read at the specified
                        interval
5  END_SYSEX            (0xF7)

This could be extended (simply by ORing the 3rd byte) to specify the interval by individual port if necessary in the future, however at the cost of added memory usage. Contrast with the proposal in my previous comment in which it would be difficult to extend the protocol (if readInterval was left out for v1) without breaking backwards compatibility in a client implementation (due to the addition of a parameter).

@soundanalogous
Copy link
Member Author

I'm declaring the Firmata serial message protocol as final.

Punted:

  • Consider adding param to SERIAL_CONFIG to optionally set data, parity and stop bits (use single byte approach like Arduino Serial lib)
  • Consider adding the ability to specify the sampling interval either per serial port or for all ports (per port will require more memory - 32 bits per serial port + the overhead of checking millis() per iteration of loop per active port ([~2us per active port per iteration of loop]

Last step is to incorporate the serial code (or a subset of it) into the StandardFirmataEthernet example.

*/

#include <Servo.h>
#include <Wire.h>
#include <ArduinoUnit.h>
Copy link
Member Author

Choose a reason for hiding this comment

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

TODO - remove this include as it's no longer needed

@soundanalogous
Copy link
Member Author

Adding Firmata serial support to StandardFirmataEthernet consumes 99% of available program memory on a Leonardo or Yun. We have reached the limit here.

Options:

  • Don't include serial support in StandardFirmataEthernet (may confuse users who expect it since it's included in StandardFirmata)
  • Create separate StandardFirmataEthernet example that includes Firmata serial support (yet another StandardFirmata variant to maintain)

The second one may be better although it adds complexity in maintaining an additional StandardFirmata variant (a change in one must be manually propagated to all unless I get creative with some scripting).

@soundanalogous
Copy link
Member Author

This is also why I was previously pushing ConfigurableFirmata for new features, but it seems the consensus is users want an all-in-one approach rather than a build your own for your setup approach (considering the limitations of any given setup).

@soundanalogous
Copy link
Member Author

I think I have a plan:

Keep StandardFirmata as it was in Firmata 2.4.4 and create a new version StandardFirmataPlus. The "Plus" indicates that it contains new features. Serial will be the first "Plus" feature and a list of "Plus" features can be maintained so users understand what additional functionality to expect when using a Plus variant. Small updates and bug fixes would still be applied to all StandardFirmata variants but larger features would only be added to the "Plus" versions. This also enables me to keep a cleaner, simpler version of StandardFirmata without all the #ifdefs, and additional complexity the Serial feature added.

StandardFirmataPlus, StandardFirmataEthernetPlus, will not be guaranteed to run on boards with <= 32K for Flash and < 3k of RAM (so Uno, Leonardo, etc will be use at your risk and in some cases it won't even compile... you'll get a Flash memory exceeded error). They will run better on newer architectures such as Due, Zero, Teensy 3.x series, etc and will also still run on Mega since it has much more Flash and RAM than the ATMega328p and ATMega32u4 based boards.

Revert StandardFirmata to 2.4.4. StandardFirmataPlus will be used
for new features. StandardFirmata will be maintained and minor
features may still be added as long as Flash, RAM and performance
of lower memory boards (Uno, Leonardo, etc) is not affected.
@soundanalogous
Copy link
Member Author

StandardFirmataPlus and StandardFirmataEthernetPlus have been added. I had to strip out the Yun code from StandardFirmataEthernetPlus since the sketch is too large to run on the ATMega32u4 of the Yun. Going to run tests on a few more boards and then merge this early this week.

@soundanalogous
Copy link
Member Author

Successfully tested:

  • Read 3 HW serial ports simultaneously
  • Read 2 SW serial ports (need to call listen method to switch between them)
  • Read 1 HW serial port and 1 SW serial port simultaneously
  • Read 3 HW serial ports and 1 SW serial port simultaneously (current maximum simultaneous reads supported)

I think this is good to go! Merging...

soundanalogous added a commit that referenced this pull request Oct 7, 2015
initial implementation of Serial protocol
@soundanalogous soundanalogous merged commit 4803a08 into master Oct 7, 2015
@soundanalogous soundanalogous deleted the serial branch October 7, 2015 06:08
@rwaldron
Copy link
Contributor

Somehow I missed that this landed—this is really exciting. Thanks for all your work @soundanalogous!!

@reconbot
Copy link
Member

This is a great plan and great work! I'm sorry I missed this too, glad you kept such good notes!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants