-
Notifications
You must be signed in to change notification settings - Fork 516
Add FirmataMarshaller::sendCapabilityQuery #330
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
Conversation
Seems useful but I would constrain additions to "core" functionality and not venture into anything like Servo, I2C, etc. |
I understand your concern and the ultimate direction is completely up to you. However, the advantage of a pure marshaller, is that its sole purpose is to serialize a function along with its parameters into the protocol and ensure everything is well formed. Therefore, I would expect for the marshaller to have near complete coverage of your protocol. If any logic or state were required to sequence, aggregate or otherwise make sense of the calls, that would belong in FirmataClass or some other implementation that takes advantage of the marshaller and parser. |
The issue is that boards like the Uno and Leonardo are near capacity with StandardFirmataPlus. This is also the reason for creating ConfigurableFirmata, to provide a wider range of features while allowing the user to pick a selection that will run on the limited memory of their chosen platform. Therefore I think FirmataMarshaller should be limited to the API described in the protocol.md file minus the other files linked at the end. I'm framing this as the "core" set of Firmata features. All other features are "optional" or "contributed" and will have their own versioning scheme going forward. The protocol version will only apply to the features described in protocol.md. Things like |
Other features could have their own marshaller classes. The following could be included in both firmata/arduino and firmata/ConfigurableFirmata:
But only ConfigurableFirmata would have the full set (including additional features like Stepper, OneWire, RotaryEncoder, etc). |
I've been thinking more about this, and having all the calls in the marshaller should not bloat the code. I think the compiler and linker will ignore any code that's not called. Therefore, your different manifestations of the FirmataClass object would be of different sizes, but they could all include and link against the same marshaller. An easy test would be to check the size of the binary before and after this commit. They should be the exact same, because nothing in FirmataClass calls this method. Now the parser is a different story, because it would have no way of knowing what is going to be used at compile time. |
My biggest concern is around scalability. The changes I'm making in the protocol will enable users to more easily add new "optional" features. There could be 10s or hundreds at some point. Would you want to see them all added to FirmataMarshaller? There could also be more than one implementation of a type of feature. There could be 3 different I2C implementations for example. |
Also most of those features will never be implemented in Firmata, they only will in ConfigurableFirmata. Maybe ConfigurableFirmata and Firmata just need separate FirmataMarshaller implementations then. |
FirmataMarshaller should follow the protocol and not the implementation. Therefore, if the protocol were to support three variations of I2C, then the marshaller would formalize those calls. In your example, I would expect FirmataClass (or some other consumer of the marshaller and parser) to leverage the same protocol (or marshalling calls) to make three different implementations of I2C. In any case, the compiler and linker will leave any unused methods out of the compilation, and the binary will stay small. |
Yes but I'm planning on divorcing "optional" features from the core Firmata protocol. I was going to do this for the Firmata protocol 2.6 release, but am beginning to think it's probably drastic enough to call it a protocol 3.0 release. What I'm aiming for is that the "protocol" defines the part of Firmata that aligns with the midi spec (and would still work with a midi parser) and some functionality that uses sysex to query the state and capabilities of the board as well as extend some of the parts where midi commands fell short (reading and writing analog pins > 15 or resolution > 14 bits for example). Anything that defines the interface for a generic or specific component (Servo, I2C, OneWire, Stepper, etc) will have it's own version. One way to think about how these features are defined in firmata/protocol (or elsewhere) instead as "interfaces" that happen to use the Firmata sysex message scheme (as opposed to part of the Firmata protocol). Some of these interfaces would be defined in firmata/protocol, each with a separate readme file (as they currently are but will be versioned independently). Others will be contributed and hosted and defined outside of firmata/protocol and the only documentation in firmata/protocol for those features would be the allocated feature ID. Some optional features will still be defined in firmata/protocol but each will have it's own version. Users will also be able to contribute features that will not be defined in firmata/protocol other than reserving a feature ID. |
This is much like how the interfaces for communicating with midi equipment special features over sysex is not part of the midi protocol. Sysex is defined, but each manufacturer is responsible for defining how to interface over sysex to some extent. |
I think we are actually on the same page, but I didn't understand the direction you were trying to take the protocol (which is great - I happen to be a purist myself). If I'm understanding you correctly, the FirmataMarshaller will in fact follow the protocol, but the surface area of the protocol will change as you categorize the functionality. You intend to create a strict MIDI interpretation that will handle the core functionality. Whereas in other cases, such as I2C, each category would have it's own "protocol" (term used loosely) and therefore its own marshaller. Is my understanding correct? |
Yep that's correct. |
@zfields did you want to keep adding to this PR? |
Sorry for not getting back to you more quickly. I'm "down home" for Christmas, and there isn't internet everywhere out here. A merge won't phase me, I'll migrate my changes (if I have any) to a new branch and make another PR if necessary. Thanks for checking! |
same code was included in #345 |
Beginning to expand the marshalling layer. This does not break backward compatibility or disturb the
Firmata.h
API surface.