Split marshalling behavior into new FirmataMarshaller class

[zfields]

This new class will provide cross platform marshalling of Firmata procedure calls. It can be updated independently of Firmata.h, which allows its functionality to be enhanced without breaking the existing API surface offered through Firmata.h.

This passes the Firmata test suite, as well as my independent verification using an UNO and Windows Remote Arduino.

[soundanalogous]

Your proposed changes make sense. I'm trying to think of how this would look as a stand-alone library (that firmata/arduino uses).

Standalone lib would contain:

• FirmataParser
• FirmataDefines (I'd rather this be called FirmataConstants)
• FirmataMarshaller
• A README describing the role of these files and how they should be used (pointing to Firmata.cpp/Firmata.h in firmata/arduino and Windows Remote Arduino as examples)

Also curious if FirmataMarshaller would make more sense as a FirmataBase class. I guess the issue there would be that some of the method signatures differ slightly (size_t vs byte).

[zfields]

I will change FirmataDefines to FirmataConstants, because I think that matches the intent and doesn't necessarily need to describe the implementation.

I wouldn't expect an Arduino user to use these new classes, unless they were intending to control a subsequent arduino or arduinos (an interesting scenario). I would mostly expect to see this on Raspberry Pis, CHIP, etc.

I don't see it as a base class because it strictly translates calls from stubs into your protocol, which is the literal definition of marshalling. In this way the marshaller can be stateless (with the exception of a stream - which I should eliminate). If you are concerned about duplicated effort, v3.0.0 would allow you to shuffle certain methods (i.e. sendValueAsTwo7bitBytes) from FirmataClass into FirmataMarshaller that may be better suited to implementation only. That example specifically would allow you to make a more advanced packing algorithm without disrupting your users.

FirmataClass has a unique role of orchestrating the parser and marshaller specifically designed for the Arduino, and I don't see any reason to change. Based on some of the comments and unused code, I see that you have already started to think about retaining state, in order to extend and improve the behavior of FirmataClass. That totally makes sense, and that scenario is enabled and encapsulated with this new architecture.

Let me rev this real quick and we'll talk again.

[zfields]

I made the file name change to FirmataConstants.h.

I made FirmataClass a friend, so it will use the packing logic from the FirmataMarshaller. That way if/when the packing algorithm gets more sophisticated, it will automatically propagate.

Finally, in an attempt to remove all state from FirmataMarshaller, I tried to introduce a static callback.

This caused me to battle with the compiler and ultimately lose. ;-P Then that thrust me into a giant philosophical debate (with myself) about why static callbacks aren't the way to move forward. As you know, the current callbacks in the parser are static, likely due to the fact FirmataClass was built for an Arduino programming environment that uses extern static members for all its classes.

Now that we are moving components (parser and marshaller) into cross platform libraries that will be executed on more powerful systems, it seems silly to force them to only have a single instance. For example, there is no reason a Raspberry Pi 3 shouldn't be able to send data to and receive data from two MKR1000s using WiFi simultaneously.

In summary, leaving the Stream member in FirmataMarshaller provides it the flexibility to send data to multiple endpoints, so I left it alone. Then later, perhaps v3.0.0, we can introduce a void * context argument to the signature of the parser callbacks and this would provide the flexibility needed to support multiple recipients.

[soundanalogous]

nullptr issue with Intel compiler

[soundanalogous]

Also used a few other places in this file.

[zfields]

Doh! Sorry that's normally a good habit! :-P

fixing...

[zfields]

fixed

[soundanalogous]

I'll run some smoke tests tonight.

[zfields]

@soundanalogous Sounds great! I'll check in periodically this evening to see if you hit any snags.

[soundanalogous]

Also, where is the best URL to direct users to for Windows Remote Arduino? I don't currently have a link to it in the README file.

[zfields]

Thanks for thinking of that! Since I'm rev'ing the core as we speak, let's make that update later.

[soundanalogous]

remove semicolon or this won't compile

[soundanalogous]

Other than this I'm not seeing any issues when running my test rig and complies without errors for all supported architectures.

[zfields]

One moment please.

[zfields]

@soundanalogous fixed

[soundanalogous]

I think all is good here. @zfields anything else you wanted to add to this PR?

[zfields]

@soundanalogous For now, I think it accomplishes what I set out to do. I would love to continue to contribute as much as you'll let me. I love the protocol and I believe with minimal tweaking it will allow me to achieve all the goals I have for my project for the foreseeable future.

[soundanalogous]

These 3 deprecated defines need to be included in Firmata.h or older Firmata sketches will break.

[zfields]

I meant to leave those in there. Let me see what I've done.

[zfields]

I originally left them in there. I must have removed them by accident, I'll put them right back. I'm sorry.

[zfields]

@soundanalogous The deprecated defines are restored, and it compiles and passes my tests

[soundanalogous]

Cool all looks good now. Merging...

To replicate the compiler test sequence I go through, install the board packages for the following packages using the Arduino boards manager:

• Arduino SAM Boards
• Arduino SAMD Boards
• Intel i586 Boards
• Intel i686 Boards
• Intel Curie Boards
• chipKIT
• esp8266 by ESP8266 Community

You may need to enter the following URLs into Arduino > Preferences... > Additional Boards Manager URLs to get some of the above packages to show up:

https://github.com/chipKIT32/chipKIT-core/raw/master/package_chipkit_index.json
http://arduino.esp8266.com/stable/package_esp8266com_index.json

And also install the TeensyDuino library (It's typically 1 version of the Arduino IDE behind latest stable is for Arduino 1.6.11): http://www.pjrc.com/teensy/td_download.html

Then try to compile StandardFirmata or StandardFirmataPlus for the following boards:

• Arduino Uno
• Arduino Mega
• Arduino Leonardo
• Arduino Zero
• Arduino Due
• Arduino 101
• Intel Galileo v2
• Intel Edison
• Adafruit HUZZAH ESP8266
• Teensy 3.6
• Teensy 3.1/2
• Teensy 2.0

Also:

• Compile StandardFirmataWiFi against the Arduino MKR1000 and HUZZAH ESP8266
• Compile StandardFirmataBLE against the Arduino 101.
• Compile StandardFirmataChipKIT against the chipKIT uC32

Now that I think of it, it's may be possible to script this process using arduino-builder. I'll try to look into that later this week or next weekend.