protocol icon indicating copy to clipboard operation
protocol copied to clipboard

first draft of feature query proposal

Open soundanalogous opened this issue 8 years ago • 20 comments

The proposal is pretty well described in the markdown file. I've also added some inline comments regarding parts I'm not 100% sure of yet.

soundanalogous avatar Jul 16 '17 21:07 soundanalogous

@zfields I'd love to get your feedback on this proposal when you have some time for a review.

soundanalogous avatar Jul 16 '17 21:07 soundanalogous

@soundanalogous Work is heavy, but I'll look into before the weekend. I just skimmed, I liked a lot of what I saw, but I would like to give this proposal a good hard look.

zfields avatar Jul 18 '17 06:07 zfields

@zfields take your time. I'll be traveling until early August.

soundanalogous avatar Jul 19 '17 03:07 soundanalogous

@soundanalogous First pass at feedback is in place. Looking forward to your response. Cheers!

zfields avatar Jul 31 '17 15:07 zfields

@zfields I'm back from vacation and will review your feedback this weekend.

soundanalogous avatar Aug 04 '17 14:08 soundanalogous

@zfields I've responded to your round 1 feedback.

soundanalogous avatar Aug 05 '17 21:08 soundanalogous

A note about backward compatibility. I think it is awesome when a project can provide backward compatibility, but it is big, slow and expensive to maintain. Due to the nature of our library and the platforms it runs on, I think v3.0.0 best serves our future audience by not being constrained to previous architectural decisions of v2 (the exception being the version query should remain backward compatible). However, I feel strongly that we should move to v3.0.0 through work done to improve v2 until such a time that the necessity to depart to v3.0.0 becomes obvious.

zfields avatar Aug 13 '17 02:08 zfields

In that case it's safe to abandon the EXTENDED_ID idea since no EXTENDED_IDs were ever allocated and there are enough bytes remaining for v2 to run for a while before moving on to 14-bit ID scheme in v3.

So a firmware query in Firmata v2.x would return the following for each feature the firmware implements.

FEATURE_ID (the FEATURE_DATA value as defined in FirmataConstants)
FEATURE_MAJOR_VERSION  (0-127)
FEATURE_MINOR_VERSION  (0-127)

soundanalogous avatar Aug 13 '17 02:08 soundanalogous

We might need some kind of "Software feature query", to get extended capabilities that are not pin-related. This might include querying whether some "software only" features are available (i.e. the FirmataScheduler) but also related capabilities for these, such as the amount of available flash or ram on the chip. I'll think of a new proposal when I have the bigger picture.

pgrawehr avatar Jul 19 '21 09:07 pgrawehr

It doesn't make sense to have anything regarding software in the protocol directly, because the list could be potentially enormous and quickly bloat the protocol.

To capture software features, it makes more sense to have a protocol within a protocol, and have some form of Sysex request, that could offer a Sysex response of implemented software features. This would move it to a higher level request, and allow the base to remain minimalist and performant.

zfields avatar Jul 26 '21 15:07 zfields

Clearly this would need a big enum (14 bits or more) for the features. But then it doesn't make a big difference whether it's a sysex command or a "top level command", at least not from a conceptual point of view. Since supposedly these query commands won't be time critical, the length of the request is rather irrelevant.

pgrawehr avatar Jul 26 '21 16:07 pgrawehr

Firmata is a hardware description protocol. I don't believe it should be expanded to support arbitrary software features.

Software is an orthogonal space, and I can't see the long-term benefit in supporting it.

@pgrawehr, I'm assuming you've given it a bit of thought. Would you mind to elaborate on your position and make a case for software support as well as describe the benefit?

zfields avatar Jul 26 '21 16:07 zfields

@zfields I was working on this quite big extension (Name is temporary). It's basically a C#/IL interpreter that uses firmata as communication and hardware backend. Conceptually, it's similar to the FirmataScheduler, except that it is much more powerful. Downside is that it requires large amounts of RAM and Flash (and therefore currently requires an Arduino Due or ESP32).

For this, I would like to query how much flash there is, how much RAM there is, and possibly a bunch of other hardware properties (little endian/big endian, sizeof(int), etc). Of course, I could add that to the sysex commands of my library, but others might benefit from it as well. Also, there's currently no way of detecting whether the pure software features (such as FirmataScheduler, or also my extension) are installed other than to request a feature and wait for a timeout. A command that always returns an answer, even if it is "I don't know what you're talking about", would help.

pgrawehr avatar Jul 26 '21 17:07 pgrawehr

I agree that we should not be waiting on a timeout. I would expect there is a switch somewhere that selects the appropriate response when available, and is electing not to respond when it could easily send a NACK or something along those lines.

zfields avatar Jul 26 '21 17:07 zfields

The problem here is that the firmata protocol doesn't require that every command gets a ACK/NACK reply. Particularly the basic commands don't require one (nor even define one). This could be made mandatory, but this is a breaking change and might cause a performance penalty (acking all GPIO commands is probably not desired).

While it is clearly a programming error, getting a better feedback when you try a command that's not supported (because the corresponding module is not available) would probably be helpful.

pgrawehr avatar Jul 26 '21 18:07 pgrawehr

I was using NACK as an example. It would be very expensive to ACK/NACK every message.

Assuming there is a switch that can recognize a particular feature, then that switch should also be capable of returning an indicator that the particular feature was not recognized. I don't believe there should be any sweeping change to the protocol. Only a very surgical modification to eliminate the experience of waiting for timeout.

The concept of "software features" is unique to Configurable Firmata (based on what you're saying) and your particular application.

I think we need a more creative compromise to ensure your application is responsive and robust. I am in support of extending an existing pathway to enable your application, but I think it would be a mistake to create a dedicated pathway.

zfields avatar Jul 26 '21 19:07 zfields

My main point here is to phase out the use of the capability query for non HW specific features. Basically if it's not a true HW capability of the pin it should not be reported. By that logic, even early additions such as Servo should be phased out and replaced by another mechanism. It's just not going to scale the way we've used capability query in the past to keep assigning new "pins" for new features.

soundanalogous avatar Jul 26 '21 23:07 soundanalogous

@soundanalogous Yes, I am aware of the original intent of this proposal. I'm just trying to get a picture on whether that's really bringing us forward the way it's currently designed. For most new features, the use of new (artificial) "pin modes" has worked quite well, since many of these features work only on some pins. And we're far from exhausting the number of possible modes. That's why I brought the discussion towards those features that cannot be assigned to pins at all.

@zfields Maybe we could make this configurable (either by command or by firmware switch)? We define a new ACK/NACK message type and if the switch is enabled, we send a reply on each unrecognized or erroneous command.

pgrawehr avatar Jul 27 '21 08:07 pgrawehr

I'm a big fan of making it configurable in some fashion. I do realize the importance of these features, and there must be a clever compromise of capability and complexity.

It's probably obvious, but my stance is much more aligned with...

"early additions such as Servo should be phased out and replaced by another mechanism."

However, this is precisely why I believe in the need for a clever, compatible and altogether new approach for integrating software features. Let's see if we can accomplish our goals by leveraging/extending an existing component of the protocol, such as sysex transactions (or something similar).

zfields avatar Jul 27 '21 18:07 zfields

You mean because servo is abusing "top level" commands? I haven't worked with this component yet, so I wasn't aware of this indeed not optimal approach. So yes, I agree that this should probably be deprecated. I was referring to having pin modes such as "DHT" or "Tone", which is IMHO fine.

Sysex transactions... Idea: We define a special Sysex command that is a "SysEx extra header". Something along the lines of:

0  START_SYSEX       (0xF0)
1  SYSEX_EXTRA_HEADER(0x10)
2  TRANSACTION_ID_LO (byte)
3  TRANSACTION_ID_HI (byte)
4  TRANSACTION_FLAGS (bit 0: Requires ACK, bit 1: Send NACK only, ...)
5  START_SYSEX       (0xF0)
... (original message)

If this is prefixed, the parser will ensure the transaction scope. The components wouldn't need to be changed, as we'd just call the handleSysex method with a pointer to the remainder only.

pgrawehr avatar Jul 27 '21 19:07 pgrawehr