firmata / protocol

Documentation of the Firmata protocol.
993 stars 152 forks source link

Concept of `FIRMWARE_RESPONSE` is not distinct from `REPORT_FIRMWARE` #74

Open zfields opened 7 years ago

zfields commented 7 years ago

This is different from the rest of your calls, because it puts the same key that was used to invoke it back on the wire. Same goes for protocol versioning.

It's possible to work around, but it tripped me up, so I thought I would call it out.

soundanalogous commented 7 years ago

Yep, same issue with PROTOCOL_VERSION. These are things I intend to clean up in Firmata 3.0.

zfields commented 7 years ago

Have you given thought to how you intend to handle it? I would like to program against it (roughly), if you have an idea. Will there be additional control byte(s), or will it follow the firmware sysex response model?

zfields commented 7 years ago

I noticed it because the version callbacks only work well enough to preserve the original FirmataClass implementation, but are insufficient for a final solution. Since the request/response is ambiguous, the callback simply notifies that a version request has been made, but does not indicate any version information that may have been passed.

Here are the response callback related signatures I had considered for my work:

typedef void (*versionResponseCallbackFunction)(size_t major_version, size_t minor_version, char * name, void * context);

void attach(uint8_t command, versionResponseCallbackFunction newFunction, void * context);
soundanalogous commented 7 years ago

For the firmware protocol message, I could allocate a separate ID for query and response, keeping the existing ID for response and allocating 0x74 for the query.

I had some thoughts regarding the protocol version here: https://github.com/firmata/protocol/issues/1. There is a separate issue there as well in which 0xF9 throws an error in webmidi since it's a reserved sysex command byte.

soundanalogous commented 7 years ago

TODO: add new QUERY_FIRMWARE_VERSION and QUERY_PROTOCOL_VERSION commands. Also deprecate 0xF9 because it's not midi compliant and allocate a new sysex message instead.

dimitry-ishenko commented 7 years ago

@soundanalogous out of curiosity, why did you choose to use midi protocol and want to be compliant with it?

soundanalogous commented 7 years ago

Midi was chosen for simplicity. That was back in 2006 or 2005 when Firmata was initially created. I was not involved with the project at that time. The reason I've been maintaining midi compliance is so that Firmata client libraries can make use of existing midi parsing libraries as a convenience. In Firmata 3.0 we may break this rule.

dimitry-ishenko commented 7 years ago

Fair enough. Is there a doc somewhere to see what's coming in Firmata 3.0?

soundanalogous commented 7 years ago

We need a doc. So far there are just some milestone tags, but it's not up to date and is spread across the firmata/protocol repo and the firmata/arduino repo:

https://github.com/firmata/protocol/milestone/2 https://github.com/firmata/arduino/milestone/7

Not definite timeline yet either. The core contributors have been slammed with regular day-to-day work. Once we all find time to converge on a plan we'll have a better sense for a 3.0 timeline.