Individual Pins Reporting Interval

[mdlima]

Based on my particular use case - using moisture sensors (see this Soil Moisture Sensor Hookup Guide) - and some other issues opened here and at firmata.js, I propose we enable the possibility of individual reporting intervals for each analog pin being reported. The current behavior (reporting all at the sampling interval) can still be the default and even simultaneously present, which means that some pins may have separate (longer) reporting intervals and some may be reported in the main loop at the global sampling interval.

For that to happen, a new message is required. I can see 3 alternatives, which I open here for discussion:

1- Updating report analog pin (0xC0) to use the first and second byte to pass the reporting interval;
2- Piggyback on the proposed EXTENDED_REPORT_ANALOG (SYSEX 64H) and use the bytes after the pin number to set the reporting interval;
3- Create a new SYSEX feature used specifically for this, maybe called ANALOG_REPORT_CONFIG.

1 is simple but a breaking change that would have to wait for v3.0.

2 seems to be a good balance, but I'm not sure why EXTENDED_REPORT_ANALOG and what are the challenges there, so could be a blocker. Also, I only have Unos and Nanos, so I wouldn't be able to test the extended port range.

3 is fine too and could be expanded to include more settings, like a linked digital pin to power the analog sensor only when the reading is done and/or a custom delay (see this thread).

[soundanalogous]

My preference here is actually to add a new ANALOG_READ message. That would enable the client (firmata.js in your case) to control the interval. The value would be reported back using the standard ANALOG_MESSAGE. The advantage here is it saves a lot of RAM. If I were to enable reporting at the individual analog pin level, I'd need to allocate 7 bytes per pin (4 bytes for the timestamp, 2 for the interval and 1 for the pin number). Dynamic memory allocation is generally not safe for boards with very limited RAM so I'd need to pick a maximum number of pins that can report individually and statically allocate an array. Say that's 16 pins so I need to allocate 112 bytes. If I simply add an ANALOG_READ message, there is very little memory impact, but it is not as accurate as having the firmware control the interval (however I'm not sure that's a big issue for analog pin reporting - for digital it would be).

[mdlima]

I see, makes sense. I implemented it and had it running before you replied (here's the diff). As you said, I had to replace the int analogInputsToReport by two arrays of size TOTAL_ANALOG_PINS, one of unsigned ints and the other of unsigned longs, so quite an increase in memory usage. It tested fine here, but I'm using a Nano, which doesn't have a bunch of analog ports. I can see how this would be a problem though.

So in your proposed solution we would be effectively creating a new ANALOG_READ message that doesn't enable reporting, right? I think it makes sense as the added message doesn't add more chatter to the serial comm as long as the pins querying frequency is less than half the global reporting frequency.

Since this will probably be a SYSEX message supporting variable commands and sizes, do you have anything against adding extra inline settings for delayed (reference) and smoothed (reference) readings?

Does Feature ID 7DH EXTENDED_ANALOG_READ sounds like a good one?

Here's the proposed message expansion:

0  START_SYSEX             (0xF0)
1  EXTENDED_ANALOG_READ    (0x7D)
2  READ_NOW                (0x01) (having a sub command leaves space for adding new ones)
3  pin                     (0-127)
4  read average loops      (optional - does multiple analog reads and returns the average value)
5  read delay              (optional - delay in us to wait before reading)
6  power pin               (optional - sets the power pin to high before reading and to low afterwards)
N  END_SYSEX               (0xF7)

[soundanalogous]

Some questions regarding your proposal:

• What is your use case for read average loops?
• What is your use case for read delay?
• What is your use case for power pin? This really should be a separate Firmata message.

I could consider read delay if there is a good use case. I'm not sure about read average loops, that may fit better with this proposal.

Here is my preference for adding ANALOG_READ:

// The client must request an analog read, then the client will respond
0  START_SYSEX              (0xF0)
1  EXTENDED_ANALOG_READ     (0x66)
2  QUERY                    (0x00)
3  pin                      (0-127)
4  END_SYSEX                (0xF7)

// In order to report analog values for pin number > 15 and/or values > 14 bits
0  START_SYSEX              (0xF0)
1  EXTENDED_ANALOG_READ     (0x66)
2  RESPONSE                 (0x01)
2  pin                      (0-127)
3  bits 0-6                 (least significant byte)
4  bits 7-13                (most significant byte)
... additional bytes may be sent if more bits are needed (could limit this to 32 bits)
N  END_SYSEX                (0xF7)

// Rename existing EXTENDED_ANALOG to EXTENDED_ANALOG_WRITE
0  START_SYSEX              (0xF0)
1  EXTENDED_ANALOG_WRITE    (0x6F)
2  pin                      (0-127)
3  bits 0-6                 (least significant byte)
4  bits 7-13                (most significant byte)
... additional bytes may be sent if more bits are needed
N  END_SYSEX                (0xF7)

[soundanalogous]

Another option for the QUERY:

// The client must request an analog read, then the client will respond
0  START_SYSEX              (0xF0)
1  EXTENDED_ANALOG_READ     (0x66)
2  QUERY                    (0x00)
3  pin                      (0-127)
4  readContinuously         (0 | 1) [optional] combines reporting and QUERY. default = 0 (OFF)
5  END_SYSEX                (0xF7)

[mdlima]

Sure, let me detail the use cases:

• What is your use case for read average loops?

Average smoothed reading is useful for cancelling out noise in the Vcc line. Let's take the thermistor for example. It's a variable resistance that changes with temperature and we have to do a voltage divider with a fixed resistor to read the voltage drop on the thermistor using the analog port. If you have noise on the power line, which most applications probably do, you can see jumps in the readings (in my tests as high as 10%). When dealing with this, I found this that recommends doing multiple readings and averaging the result or using the 3V3 line as it's less noisy. I'm not sure how this would apply to other analog sensors, but it seems expectable to me to have noise on analog signals.

As for the separate proposal, I agree that this can fit there, but if you agree that most analog circuits might have a bit of noise and doing the smoothed reading can improve the reading accuracy, I'd say this is more of a general case that will be useful on a larger scale, so seems to fit better with the more general message than with a specific one tailored for special cases that need sampling functions. Of course I'm really biased here, so your feedback is most welcomed.

• What is your use case for read delay?

Read delay is something that I found linked from an issue on Firmata actually. Haven't really had to deal with it, as it seems to impact more in case you have a high impedance circuit feeding the analog ports. More info can be found here, but basically it can take a little to charge the capacitor used on the AVR's ADC. Here's an excerpt from it's Datasheet:

"The ADC is optimized for analog signals with an output impedance of approximately 10 k? or less. If such a source is used, the sampling time will be negligible. If a source with higher imped- ance is used, the sampling time will depend on how long time the source needs to charge the S/H capacitor, with can vary widely. The user is recommended to only use low impedance sources with slowly varying signals, since this minimizes the required charge transfer to the S/H capacitor."

On the linked thread, it's mentioned that for a circuit with a 100k ohms resistor a 10us delay is already needed, so this would probably be the case for soil moisture sensors too.

It's worth mentioning that this seems to be more necessary if you're doing multiple analog readings in sequence without any other logic between, which is the case for the smoothed (averaged) reading above but also for Firmata's report analog looping.

• What is your use case for power pin? This really should be a separate Firmata message.

This is again related to the thermistors, that you wouldn't want them to be charged the whole time as they'll be dissipating power and heating as a consequence. I need multiple samples per second as I'm using them to monitor a heatsink temperature and control its fans speed. Turning power on and off using DIGITAL_WRITE messages would add a lot of chatter to the serial communication, but of course I can work my way around it if you think this is not a replicable use case.

[soundanalogous]

Okay, I'll need to think more about the read average loops case, but it still seems a little out of place for a general purpose ANALOG_READ message. If I had heard many users asking for this over the years I may feel more strongly about it, but AFAIK you're the first person to bring it up.

I understand the issue for delay read now. However I don't think that is an issue when using an ANALOG_READ message since you would not be able to do two reads fast enough over the transport. It's an issue that occurs when you switch between 2 analog channels too quickly. This can currently happen in the analog reporting loop when multiple analog pins are enabled. If anything it needs to be part of the ANALOG_REPORTING api.

The power pin case does not fit well with the general analog read use case. It definitely needs to be a separate message or just handled using DIGITAL_WRITE messages.

[mdlima]

Fair enough, I will implement the other use cases as user messages.

I opened the pull request firmata/ConfigurableFirmata#80 for this. Some things that came to mind:

• Since the QUERY message can toggle reporting on/off, I wasn't sure what to do in case of a message with readContinuously = 0, so I decided to replicate the behavior of the report analog pin (0xC0) message which doesn't send the value and just turns reporting off. Seems to me that this will be useful for boards with more than 16 analog pins that can use this as the reporting API. Another option would be to add a subcommand specifically for toggling reporting on/off and always send the value for the QUERY subcommand.

• Do we really need the RESPONSE (0x01) byte in the report message? It would make it more extensible, but I can't see what other kind of messages could be returned and removing could save a byte in every response message. If you want to keep it, can we change the RESPONSE code to 0x00 and the QUERY to 0x10? This way they have different ranges and helps with my OCD 😄 .

• Since the number of value bytes in the response was flexible, I stretched the concept so that we can even have 0 bytes (in case the value is 0) or just 1 byte in case the value is between 63 and -64. I wasn't sure about negative values here, so I decided to support them as this can then be used for other messages that include negative values.

[soundanalogous]

Overall I'm not yet set on this proposal. It may take some time to settle. Simpler is better to start.

To answer your questions:

Since the QUERY message can toggle reporting on/off

I'm not set on including the reporting ON/OFF parameter yet. It was just a suggestion.

Do we really need the RESPONSE (0x01) byte in the report message?

Yes since it's helpful for callbacks in the firmata/arduino implementation that can work as both firmware and as a C++ client implementation.

can we change the RESPONSE code to 0x00 and the QUERY to 0x10

No because the general pattern is to enumerate commands starting from 0x00. I veered from this once and ran into scalability issues (see Firmata Serial v1.0) so I want to stick with the standard enumeration scheme. It's probably safe here, but I'd rather stick with the convention.

I wasn't sure about negative values here, so I decided to support them as this can then be used for other messages that include negative values.

I'm not sure if negative values are useful. If there is a good case for them, then the first step is to establish a good standard for using negative values throughout the Firmata protocol. My initial inclination is to adapt the format used in AccelStepperFirmata. However as I said, I'm not yet convinced this is useful or at least not included in v1.

[mdlima]

Overall I'm not yet set on this proposal. It may take some time to settle.

Too bad, I was hoping to get this merged so I can open the PRs on firmata.js and johnny-five, as I've got all the code ready. But I get it, protocol changes are always tricky... Which makes me think, do you have a good standard for deprecating messages so that you can evolve the protocol continuously? Because if not, this is always going to be a hindrance. The best example I can think of lately is Kubernetes. Not sure if you've been following their progress but it's impressive how much it's grown and how fast they were able to push changes to their API. Not exactly the same, of course, as they sort of control both client and host, but still something to aspire to.

I'm not set on including the reporting ON/OFF parameter yet. It was just a suggestion.

Ok. Just wanted to point out that, as implemented, it's an optional param, so it works without it already. Seems to be good for the case with boards with more than 16 analog pins. From what I've seen, looks like the need for reading from higher numbered pins has been around for a while, and this solves that too, and with reporting, so could be a good win.

No because the general pattern is to enumerate commands starting from 0x00. I veered from this once and ran into scalability issues (see Firmata Serial v1.0) so I want to stick with the standard enumeration scheme. It's probably safe here, but I'd rather stick with the convention.

Like a friend always says, experience is what you get when things don't work out as you wanted, so let's benefit from it. =)

I'm not sure if negative values are useful. If there is a good case for them, then the first step is to establish a good standard for using negative values throughout the Firmata protocol. My initial inclination is to adapt the format used in AccelStepperFirmata. However as I said, I'm not yet convinced this is useful or at least not included in v1.

I saw that implementation, but it requires to always send all the 32 bits, limiting larger than 32-bit integers and always requiring the messages to have 5 bytes. This other proposition is flexible in both ways, so I thought I'd see if you'd consider.

[soundanalogous]

Are you aware of any microcontroller platforms that return a negative value for an analog read? The convention Firmata currently uses is to send the value that corresponds to the voltage. On most Arduino boards, that's an 8 bit value but some boards support 10 or 12 bit values (but still default to 8 bit). The number of bits supported per channel (pin) are reported via the configuration query and Firmata client libraries use this to know how to convert the value properly. I'm not aware of a use case for a negative value.

[soundanalogous]

I'm also not sold on the idea of a returning 0 bits. My preference would be a minimum value of 8 bits. However if there is a good case for returning < 8 bits then the we'd need another parameter to specify the number of bits returned or the client would not know how to mask the value correctly.

[mdlima]

Are you aware of any microcontroller platforms that return a negative value for an analog read?

Not really, it was a question I had too, but I also meant in a more general case. If this is confirmed to be always positive, then either the protocol must define that the return is an unsigned int or live with the max resolution of N-1 bits (N being the total bits returned in the multiple 7 bytes in the message), which is not a big loss.

I'm also not sold on the idea of a returning 0 bits. My preference would be a minimum value of 8 bits. However if there is a good case for returning < 8 bits then the we'd need another parameter to specify the number of bits returned or the client would not know how to mask the value correctly.

That is sort of my point regarding the AccelStepperFirmata implementation, that it requires the client to know exactly how many bits are being returned as the signal is in the last one. To reconstruct the value using the proposed protocol above, the client needs to know only how many bytes are carrying the value, which is done by using the END_SYSEX as a marker here. The side effect is that you can't have optional or variable parameters after this message, so it wouldn't work if you have to return multiple values for example.

Given the bytes carrying the value are stored in an Array, the logic to reconstruct would be as follows:

for (var i = 0; i < numberBuffer.length; i++) {
  value = value | ((numberBuffer[i] & 0x7F) << (i * 7));
}
if (numberBuffer[numberBuffer.length - 1] & 0b01000000) {
  // Last higher bit is 1, so fill the remaining bits with 1's
  value = value | (-1 << (7 * numberBuffer.length));
}

[soundanalogous]

if (numberBuffer[numberBuffer.length - 1] & 0b01000000) {
// Last higher bit is 1, so fill the remaining bits with 1's
value = value | (-1 << (7 * numberBuffer.length));
}

I'm sure I understand why filing the remaining bits with 1's is necessary.

[mdlima]

That's for reconstructing the negative numbers, using signed 2's complement.

Sending 32

16-bit binary: 0000 0000 0010 0000 [0x0020]

7-bit bytes:

   data sent       [mask & remaining]
1. 010 0000 (0x20) [0x7F & 0x0020]
X. 000 0000 (0x00) [0x7F & 0x0000 (0x0020 >> 7)] => not sent because it's all 0's and last byte higher bit was 0
X. 000 0000 (0x00) [0x7F & 0x0000 (0x0000 >> 7)] => not sent because it's all 0's and last byte higher bit was 0

reconstruct:

   accumulator         | new data
1. 0000 0000 0000 0000 | 0000 0000 0010 0000 [0x0000 | 0x20]
 = 0000 0000 0010 0000 [0x0020] => 32 (no filling required)

Sending -32

16-bit binary: 1111 1111 1110 0000 [0xFFE0]

7-bit bytes:

   data sent       [mask & remaining]
1. 110 0000 (0x60) [0x7F & 0xFFE0]
X. 111 1111 (0x7F) [0x7F & 0xFFFF (0xFFE0 >> 7)] => not sent because it's all 1's and last byte higher bit was 1
X. 111 1111 (0x7F) [0x7F & 0xFFFF (0xFFFF >> 7)] => not sent because it's all 1's and last byte higher bit was 1

reconstruct:

   accumulator         | new data
1. 0000 0000 0000 0000 | 0000 0000 0110 0000 [0x0000 | 0x60] (would result in 96, so filling is needed)
2. 0000 0000 0110 0000 | 1111 1111 1000 0000 [0x0060 | 0xFF80 (-1 << 7)] => filling unreceived bits with 1's
 = 1111 1111 1110 0000 [0xFFE0] => -32

I wrote the examples using 16-bit integers, but it would be exactly the same bytes sent if working with 32-bit.

[soundanalogous]

Okay that's what I thought you were talking about, but I was confused because it sounded like just before that you had agreed that negative numbers weren't an important use case here.

I'm inclined to support only an unsigned integer here as I'm not aware of any microcontroller platform where an analog read returns a float. If you can dig up some examples to prove otherwise I may reconsider.

Edit: Not aware of any microcontroller platform where an analog read returns a negative number.

[mdlima]

You're right, unsigned is better here. I'll update the PRs.