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

first draft of feature query proposal #91

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

soundanalogous
Copy link
Member

@soundanalogous soundanalogous commented Jul 16, 2017

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.

@@ -118,6 +118,7 @@ Following are SysEx commands used for core features defined in this version of t
```
EXTENDED_ID 0x00 // A value of 0x00 indicates the next 2 bytes define the extended ID
RESERVED 0x01-0x0F // IDs 0x01 - 0x0F are reserved for user defined commands
REPORT_FEATURES 0x65 // report which optional Firmata features are supported by the firmware
Copy link
Member Author

Choose a reason for hiding this comment

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

I could see a need in the future for having separate REPORT_CORE_FEATURES and REPORT_OPTIONAL_FEATURES where REPORT_CORE_FEATURES would not report a version for each feature since they are all covered under the Firmata protocol version. The distinction would be useful for ConfigurableFirmata where it's possible to exclude certain core features from a firmware build.

Copy link
Contributor

Choose a reason for hiding this comment

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

I would like to stay away from CORE features versus EXTENDED. The notion of core and extended are propagating the "limitations" (as it sits, not hard limitations) of the Firmata implementation into the protocol. I think the protocol should be focused on solely describing the hardware and providing access to its capabilities.
If all features were part of a feature set, then this would allow you to describe any board in a way that was conducive to ConfigurableFirmata.

Copy link
Member Author

Choose a reason for hiding this comment

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

There are hardware capabilities (GPIO, I2C, UART, SPI, etc) and then there are firmware-specific capabilities (OneWire, Stepper, Servo, Task Scheduler, etc). All of these features could be reported using a single command, or HW-specific features could be reported separately.

Copy link
Member Author

Choose a reason for hiding this comment

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

A user would then first query either hardware capabilities or all firmware capabilities (TBD), then if they need specifics for any board-level capability, query for the pins, etc.

Copy link
Contributor

Choose a reason for hiding this comment

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

I actually changed my mind while writing this...

I do like classing the features as either HARDWARE or FIRMWARE, because I don't see them as equal. I think our primary responsibility is to expose the hardware as hardware. Then, when limitations arise from the remote nature of the hardware, it becomes important to have FIRMWARE support; which I view as secondary. Theoretically, you can recreate any FIRMWARE you like if you are aware of the hardware, effectively leverage SYSEX messages, and are willing to modify the .ino (I know this is a crappy experience/solution).

So to summarize, I like HARDWARE versus FIRMWARE, instead of CORE vs EXTENDED. 😺


The feature query reduces the need to define a new pin mode for every introduced feature. Going
forward pin modes will only be defined and maintained for microcontroller specific features such as
digital and analog I/O, PWM, I2C, UART, SPI, DAC, etc. The use of pin modes for non microcontroller
Copy link
Member Author

@soundanalogous soundanalogous Jul 16, 2017

Choose a reason for hiding this comment

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

I'm still a bit torn regarding how to best support pin modes going forward. I think there are 3 options:

  1. (as proposed here, at least until Firmata 3.0), Pin modes will only correspond to features that are hardware specific. However this only partially reduces the size of the capability query response.
  2. Only have pin modes for core features that are hardware specific (e.g. Digital In/out, Pull-up, Pull-down, analog in, PWM). In this case the pins used for hardware specific optional features such as I2C, SPI, Uart, etc would need to be reported separately, most likely by querying each individual feature for its corresponding supported pins. This would scale better than option 1, but is a bigger departure for a pre-Firmata 3.0 release.
  3. Mirror Arduino pin modes: INPUT, OUTPUT, INPUT_PULLUP, INPUT_PULLDOWN (have seen this on some new hardware). This is the biggest departure from the existing functionality and would require a new solution for reporting which pins support analog input and PWM. Other hardware specific pins would be reported by a per feature query as described in option 2 above.

Currently I'm leaning towards option 2 as a good target for Firmata 3.0, but option 1 as a stop-gap.

Copy link
Member Author

@soundanalogous soundanalogous Jul 16, 2017

Choose a reason for hiding this comment

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

Another option for handling pins modes for features like I2C, SPI, UART, etc would be to not worry about them at all as far as Firmata is concerned. This assumes a user has some familiarity with their board and which pins coincide with which features. The feature query in this case would inform the Firmata client library what the firmware supports and then the only time pins modes need to be specified are when they are technically required such as setting an input pull-up. Firmata may still need Analog In and PWM for the purpose of properly configuring pins if the mode is changed without physically restarting the board (but even then it would be unwise for a user to reconfigure a pin without unpowering the board first so perhaps that is moot as well), but generally with the feature query in place I think there will be no further need for reporting pin modes for most features.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can cover all the considerations mentioned above, if we were to take a step out from the pin level detail and first have a board level description.
Assuming you are not making a tailored solution (in which case any generic Firmata implementation becomes heavy weight and less valuable). As a rule, a person writing remote code doesn't have access to Boards.h, so they don't know anything about the hardware or the board itself. They should be able to query a board for it's feature set (i.e. GPIO, analog, SPI, I2C, etc...). Then, if they are interested in a particular feature like SPI, they could query it specifically. This would allow much greater detail to be returned about a pin or set of pins (i.e. SCLK, SS, MISO and MOSI pins).

Copy link
Contributor

Choose a reason for hiding this comment

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

A scenario I would like to enable, would be a teaching application that discovered a board connected to your computer and could interactively build out its capabilities and allow a person to tinker in a classroom setting with a point and click interface.
To accomplish this, we would need to associate a unique identifier with a particular board (this would allow someone to download a schematic for a particular board). Then if you could identify which board features are available through the protocol, you could enable specific UIs capable of utilizing each of the features.
If the protocol provided the ability to drill down, then we enable full introspection, providing great detail, all with minimal message sizes.

Copy link
Member Author

Choose a reason for hiding this comment

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

They should be able to query a board for it's feature set (i.e. GPIO, analog, SPI, I2C, etc...). Then, if they are interested in a particular feature like SPI, they could query it specifically

I agree and like the suggestion of breaking out GPIO as well. This would allow us to fully deprecate the configuration query.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok yeah I totally agree. I think we need one more query though:

  • Board feature query (what you describe as "first pass above")
  • HW feature detail ("second pass" from above)
  • Firmware feature query (non board-level features supported by the firmware such as Stepper, Servo, OneWire, etc)

Copy link
Member Author

Choose a reason for hiding this comment

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

We would need a good strategy for revising the HAL if we provide a HARDWARE_QUERY. The current issue is the extent that the user can interact with the HW is constrained by the current firmware (how Firmata I2C, UART, Analog, PWM, etc is defined in the protocol and implemented). Does a HW_QUERY return an identifier for the current implementation (as described in the protocol for each supported HW feature) or does it simply return a description (from an enumerated list of microcontroller HW features) of the board regardless of how accurately it's features are implemented in firmware (if at all - SPI or DAC for example). In that case a FIRMWARE_QUERY would also be required to understand which interface is exposed for each feature, or maybe we get that in the second pass (feature id to identify implementation/interface, ss, mosi, etc).

Copy link
Contributor

@zfields zfields Aug 13, 2017

Choose a reason for hiding this comment

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

In talking through this, I believe "board" above is synonymous with HARDWARE. Perhaps we should have two query roots, both HARDWARE and FIRMWARE.

         HARDWARE                     FIRMWARE
  __________|__________          ________|________
  |   |   |   |   |   |          |       |       |
GPIO ADC DAC I2C SPI ETC       SERVO  STEPPER ONEWIRE
            ______|________                      |
            |   |    |    |            <list of pins capable
           SS  MOSI MISO SCLK             of functionality>

Hardware functionality is physically bound to pins, while firmware can be applied to a certain subset of pins capable of supporting that functionality. The right thing seems to have separate roots (illustrated above), but similar models of interaction (so if you can understand one, then you can easily understand the other).

Copy link
Contributor

Choose a reason for hiding this comment

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

Does a HW_QUERY return an identifier for the current implementation (as described in the protocol for each supported HW feature) or does it simply return a description (from an enumerated list of microcontroller HW features)?

My vote: Simply return a description (from an enumerated list of microcontroller HW features)

Copy link
Contributor

@zfields zfields Aug 13, 2017

Choose a reason for hiding this comment

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

The current issue is the extent that the user can interact with the HW is constrained by the current firmware

I think the firmware needs to be revisited... I would like to rearchitect the firmware to take on concepts of configurable Firmata (composable), as well as allowing it to operate as a library. This would make it a lot easier to add Firmata to existing sketches, instead of starting with Firmata and building a sketch into it.

6 2nd FEATURE_ID (1-127, eg: Serial = 0x60, Stepper = 0x62)
7 2nd FEATURE_MAJOR_VERSION (0-127)
8 2nd FEATURE_MINOR_VERSION (0-127)
9 3rd FEATURE_ID (0x00, Extended ID)
Copy link
Member Author

Choose a reason for hiding this comment

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

Currently there are no Extended IDs defined, but even when they start to be added I think it will be rare that a firmware will include more than 1 or 2 at a time so in most cases there will only be 3 bytes used for each feature (ID, major, minor).

Copy link
Contributor

Choose a reason for hiding this comment

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

If we were to use a drill-down approach in Firmata v3.0. Then instead of using a bit mask, we can use an integer which would provide significant room for feature growth, without the need for branching logic in the protocol.

Copy link
Member Author

Choose a reason for hiding this comment

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

My intent here was to provide backwards compatibility in the same way the midi protocol handled it when it became evident 127 identifiers was not sufficient. There is no use of a bit mask in my proposal, but there would be the need to check if the first byte for each feature ID is 0x00 or not.

Are you suggesting using something like a 14-bit identifier for all Firmata features beginning in Firmata 3.0? That would definitely scale much better.

Copy link
Contributor

@zfields zfields Aug 13, 2017

Choose a reason for hiding this comment

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

14-bit identifier for all Firmata features beginning in Firmata 3.0

Yes, I would prefer it to be 14-bits, instead of a check byte. That being said, we are at 11 and 127 seems like a long way off.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, no I'm trying to deprecate most of those modes (and the capability query as well) and instead provide a separate FIRMWARE_QUERY and HARDWARE_QUERY as we've been discussing. Then make pinMode more like Arduino pin mode where you use it to set DIGITAL_IN, DIGITAL_OUT, and various PULL_UP states.

Copy link
Contributor

Choose a reason for hiding this comment

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

The interface version could be in the follow on query

I think it should ONLY be in the follow on query. For example, if I'm not going to use I2C, then I don't want to waste bytes (time and possibly money) to learn about its version...

Copy link
Contributor

Choose a reason for hiding this comment

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

What is the advantage of separating out

2 HARDWARE_FEATURE_ID (from enumerated list of MCU features)
3 HARDWARE_FEATURE_INTERFACE (the sysex command corresponding to implementation)

They would both need to be unique, correct? Why can they not be the same value?

Copy link
Member Author

Choose a reason for hiding this comment

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

They would both need to be unique, correct? Why can they not be the same value?

It depends on if we allow multiple implementations for individual HW features. For example the current version of I2C only supports a single port and does not scale. So in that case there is a MCU feature, I2C, but a specific Firmata I2C protocol implementation that is highly constrained.

Copy link
Member Author

@soundanalogous soundanalogous Aug 13, 2017

Choose a reason for hiding this comment

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

But the version may be sufficient in that case. It sorta depends on if the HW query lands in Firmata v2 or Firmata v3. I would prefer to at least add the FIRMWARE query sooner than later to provide a way for client applications to understand the firmware capabilities when using ConfigurableFirmata without needing to keep creating pin modes to accomplish the task.

Copy link
Member Author

Choose a reason for hiding this comment

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

This also means we will have to own and enforce the Firmata HAL. There should be a single definition for I2C, SPI, DAC, UART, etc. For those features that are already implemented (I2C and UART for example, but even overhauling analog and GPIO as well), they need a new definition that is more scalable. We can't simply copy the Arduino HAL in all cases since it does not always work over a transport but we can try to align where it makes sense.

@soundanalogous
Copy link
Member Author

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

@zfields
Copy link
Contributor

zfields commented Jul 18, 2017

@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.

@soundanalogous
Copy link
Member Author

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

@zfields
Copy link
Contributor

zfields commented Jul 31, 2017

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

@soundanalogous
Copy link
Member Author

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

@soundanalogous
Copy link
Member Author

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

@zfields
Copy link
Contributor

zfields commented Aug 13, 2017

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.

@soundanalogous
Copy link
Member Author

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)

@pgrawehr
Copy link
Contributor

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.

@zfields
Copy link
Contributor

zfields commented Jul 26, 2021

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.

@pgrawehr
Copy link
Contributor

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.

@zfields
Copy link
Contributor

zfields commented Jul 26, 2021

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?

@pgrawehr
Copy link
Contributor

@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.

@zfields
Copy link
Contributor

zfields commented Jul 26, 2021

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.

@pgrawehr
Copy link
Contributor

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.

@zfields
Copy link
Contributor

zfields commented Jul 26, 2021

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.

@soundanalogous
Copy link
Member Author

soundanalogous commented Jul 26, 2021

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.

@pgrawehr
Copy link
Contributor

@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.

@zfields
Copy link
Contributor

zfields commented Jul 27, 2021

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).

@pgrawehr
Copy link
Contributor

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.

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

Successfully merging this pull request may close these issues.

3 participants