firmata / protocol

Documentation of the Firmata protocol.
987 stars 153 forks source link

DHT Sensor proposal #106

Closed mdlima closed 3 years ago

mdlima commented 6 years ago

This is a proposal to add commands for communicating with DHT sensors.

The implementation is ready in the FirmataDHT repository.

soundanalogous commented 6 years ago

I prefer to keep device specific proposals documented outside of the firmata/protocol repo. The readme for FirmataDHT should include the description instead.

I'd rather find a place here to list 3rd party Firmata feature contributions and link off to them. They can also be added to firmatabuilder.com (via a pull request).

mdlima commented 6 years ago

So just the feature-registry and change that to a link to the readme in FirmataDHT?

soundanalogous commented 6 years ago

Correct, but I also don't want to allocate 0x74H to a device. I'm trying to reserve the the higher numbers for board-specific features and some common categories (Stepper, Servo, etc mainly via the legacy structure).

My preference for allocating an ID for 3rd party, device-specific Firmata features is to use the EXTNEDED_ID (3 byte) ID. This comes from the Midi spec. However if having a 3 byte ID is too much I'm willing to discuss a single byte ID, but it should be something like 0x10. However if it's just you who will use this feature I'd use something in the 0x01 to 0x0F range.

mdlima commented 6 years ago

It was mentioned on the Optional Feature Set, that's why I thought it would be the place:

Optional features also provide APIs to interface with general components (eg: servo, stepper, rotary encoder, etc) as well as specific components (eg: DHT11, NeoPixel, etc).

The EXTENDED_ID range should be fine though, it's a slow sensor and the firmware is tweaked to the minimum report interval of 2 seconds, so there's not lots of traffic anyway. Since there's no reference to any reserved EXTENDED_ID, can you suggest something?

I've seen lots of questions regarding this sensor, there's even a backpack firmware for it, so I thought this would be useful. If you think otherwise, just close the PR and I'll run it in the user id range.

soundanalogous commented 6 years ago

At the end of the that paragraph it states:

However, any feature that wraps a specific driver, specific sensor, one-off custom component, etc should use the extended feature ID (00H nnH nnH) or should use the DEVICE_QUERY/RESPONSE API.

I need to remove the part about DEVICE_QUERY/RESPONSE API though since that was a proposal that was never maintained.

I've been torn on the subject of IDs for a few years. It's a tricky subject. With the 7 bit addressing scheme we'll eventually run out of numbers, so that's why I proposed the equivalent of what the midi spec ended up doing with EXTENDED_ID. However at the rate IDs are being allocated, I don't think we'll run out anytime soon so I think it's safe to use an ID in the 7 bit range (and EXTENDED_ID also adds complexity in parsing). I can keep 0x00 reserved in case all 127 IDs are ever used up. For proposals that wrap a specific sensor, etc the range could start at 0x10 and increment. At some point that will meet with the general purpose range that is decreasing in value from 0x5C.

Regarding your DHT11/22 wrapper however. If an implementation becomes the de-facto Firmata feature for that device I prefer that it works on the widest range of devices. It's not an issue for the API, but more so for the platform specific implementation. Currently only a few Arduino boards are supported. So if you created a module for johnny-five that depends on the ID you allocated, then users are limited to only a few different Arduino boards. You could give it a user-space ID for now and continue to iterate on it to increase the number of supported boards, and then we can give it a value in the feature registry.

soundanalogous commented 6 years ago

You could give it a user-space ID for now and continue to iterate on it to increase the number of supported boards, and then we can give it a value in the feature registry.

One thing that will help here is to make sure the API you exposed is an abstraction rather than a wrapper for a specific library. That way if you need to swap out the library for one that supports a wider range of microcontroller architectures, you won't break API compatibility.

mdlima commented 6 years ago

Thanks for all the input, I'll push forward as you suggested.

pgrawehr commented 3 years ago

Adapted and merged as in "beta" state.