Open zfields opened 7 years ago
I took a walk and thought about it...
My immediate concern can be mitigated, by passing a function pointer for a pinToAnalog()
function to the FirmataMarshaller
constructor. This would allow FirmataClass
to utilize a global function defined in boards.h
that simply wraps the macro.
static inline unsigned char pinToAnalog(unsigned int pin) __attribute__((always_inline, unused)) { return static_cast<unsigned char>(PIN_TO_ANALOG(pin)); }
Obviously, this is only a fix for the ANALOG_MESSAGE
and REPORT_ANALOG
messages. However, in order to read analog pins greater than 15, we will need an extended analog reporting solution...
Now that I've piddled around with it a little bit, I've realized it also takes a slight modification to StandardFirmata.ino
; in particular, the way it calls Firmata.sendAnalog()
.
Obviously, this would actually require changes in ALL the Firmata host implementations. So naturally I began to wonder, how much code is shared between these "examples" and would there be benefit to creating a FirmataHostCallbacks.h
or something along those lines? At first blush, it seems like would dramatically improve your firmware update story, but I'm curious why this isn't already the case.
Would love to hear your thoughts on the subject...
Extended analog exists to support pins numbers > 15 and adc resolutions > 14 bits. However what's missing is the ability to enable analog reporting for pins > 15, but I have a proposal open for that.
Actually you're right. For some reason I was thinking Extended Analog covered both analog input and analog output (PWM), but it's only for analog output. So in addition to the ability to enable analog input reporting for pins > 15, a message to send analog input data for pins > 15 or for values > 14 bits is needed.
PIN_TO_ANALOG
may actually not even be necessary (at least not in the following example). I haven't thought this through in detail yet, but I don't see why:
for (pin = 0; pin < TOTAL_PINS; pin++) {
if (IS_PIN_ANALOG(pin) && Firmata.getPinMode(pin) == PIN_MODE_ANALOG) {
analogPin = PIN_TO_ANALOG(pin);
if (analogInputsToReport & (1 << analogPin)) {
Firmata.sendAnalog(analogPin, analogRead(analogPin));
}
}
}
Couldn't simply be rewritten as (ignoring pin range and value limitation of Firmata.sendAnalog
for now):
for (pin = 0; pin < NUM_ANALOG_INPUTS; pin++) {
if (analogInputsToReport & (1 << pin)) {
Firmata.sendAnalog(pin, analogRead(pin));
}
}
With the assumption that if a bit is set in analogInputsToReport
that the pin is configured as an analog input pin.
It's also possible to check the opposite of PIN_TO_ANALOG
like this (but this would still be platform dependent):
for (pin = 0; pin < NUM_ANALOG_INPUTS; pin++) {
// analogInputToDigitalPin is typically defined in a boards pins_arduino.h file
if (Firmata.getPinMode(analogInputToDigitalPin(pin)) == PIN_MODE_ANALOG) {
if (analogInputsToReport & (1 << pin)) {
Firmata.sendAnalog(pin, analogRead(pin));
}
}
}
It would not be possible to eliminate PIN_TO_ANALOG
in the other 2 places it is used (analog mapping query response and toggling reporting according to pin mode).
...would there be benefit to creating a FirmataHostCallbacks.h or something along those lines? At first blush, it seems like would dramatically improve your firmware update story, but I'm curious why this isn't already the case.
This starts getting into ConfigurableFirmata territory.
@zfields regarding the PIN_TO_ANALOG
issue, perhaps another option is defining defaults for all of the macros in Boards.h (for example) and moving the readPort
and writePort
functions from Boards.h to Firmata.h, then Boards.h could be included with FirmataMarshaller and would simply use the default macros if not run in a microcontroller environment. Of course the code that throws the compiler error would also need to be removed from Boards.h in order for this to work.
Or another option may be to define default macros in FirmataConstants.h and leave Boards.h as is.
// in FirmataConstants.h
#ifndef PIN_TO_ANALOG
#define PIN_TO_ANALOG(p) 0
#endif
// ... and any other macros necessary to compile outside of a microcontroller environment
As a consumer, I am able to sort things out (albeit imperfectly) by leveraging the analog pin mapping.
To be honest, I didn't look too far into extended analog after I discovered it wouldn't help me. However, it would be useful, and much more simple, if extended analog didn't transform the pin number. The transformation is not necessary for the Arduino, and it causes the need for all the helper functions. I would leave the macros in place to support the classic use case, but I would deprecate them and move forward with a parallel extended analog implementation that provides support for newer more sophisticated boards.
it would be useful, and much more simple, if extended analog didn't transform the pin number
To clarify, by not transform (since Arduino transforms analog to digital internally and Firmata transforms digital to analog), do you mean when using something like extended analog, you'd prefer that pin A0 on an Arduino Uno for example is 0 or 14?
Actually my memory fails me. Arduino does not transform internally, it simply allows the user to use either the analog number or the digital equivalent: https://github.com/arduino/Arduino/blob/master/hardware/arduino/avr/cores/arduino/wiring_analog.c#L38-L98.
I think we are on the same page now, but just to reiterate...
A0
is defined as 14
on Arduino Uno, and similarly on other variants. These numbers match the actual capability query response exactly, and will work without modification as long as the protocol supports it (which it currently does not - and is what I am lobbying for ;-)).
Soap box rant: Arduino allows you to refer to the analog pins with a zero based index as well as it's actual pin number. In my opinion, this becomes confusing, especially when you do the following...
int x;
int y;
// not intuitive
pinMode(3, OUTPUT);
digitalWrite(3);
analogWrite(3, 255);
pinMode(3, INPUT);
x = digitalRead(3);
y = analogRead(3); // operates on a different pin than other commands
// intuitive
pinMode(A3, OUTPUT);
digitalWrite(A3);
analogWrite(A3, 255); // does nothing
pinMode(A3, INPUT);
x = digitalRead(A3);
y = analogRead(A3);
Using the actual pin number could work. The proposal here would still hold up in that case but would use the actual pin number rather than starting from 0.
I would still encourage Firmata client developers to provide a means for users to specify analog numbers starting from 'A0' or 0 since that is how most boards are labeled. The client library would then send the actual pin number (so 14 instead of 0 or 'A0').
I am writing (yet another) C++ client library for Firmata and I am getting confused by when to use analog vs regular/digital pin position.
(a) It is my understanding is that analog data messages (0xE0 - 0xEF) are only for receiving data from the host, correct? Not for sending. And these messages refer to the analog position of the pin.
(b) Likewise, when sending analog reporting messages (0xC0 - 0xCF) to the host, we use analog position of the pin, right?
(c) Now, extended analog messages (0xF0 + 0x6F) are used to control PWM pins and in this case we use regular/digital pin position. And these messages are only for sending, not receiving data (from the standpoint of the client). Is that right?
Sorry, I had to ask to be sure.
@dimitry-ishenko I have a good example of bi-directional analog communication on my project - Remote Wiring. I know it doesn't directly answer your question, but it's the best I could do without researching. I'll try and improve the response when I have more time.
Thanks @zafields. I took a peek, but... no cigar :smile:
@dimitry-ishenko Have you looked at the new FirmataMarshaller and FirmataParser breakout in the firmata/arduino repo. They are the logic used by StandardFirmata.ino to handle the majority of C++ messaging concerns.
OK, the thing that confuses me right off the bat: I am looking at FirmataMarshaller.cpp. This is a host side implementation of the protocol for Arduino, right? Why does it implement analog reporting on/off command? Isn't this something that is done on the client side?... Am I just smoking something bad or what?
@dimitry-ishenko
This is a host side implementation of the protocol for Arduino, right?
No. It simply marshals calls into the Firmata protocol.
@zafields whose calls? Arduino's? So, is this a client side implementation for Arduino? Sorry, if I am asking stupid questions.
@dimitry-ishenko All good questions...
The Firmata protocol supports, several messages. The marshaller should be able to provide a method (i.e. sendAnalog
) and be able to marshal it into a protocol message that can be interpreted by the parser.
Therefore, FirmataMarshaller
and FirmataParser
should be viewed as two sides of a bridge. In order to support two-way traffic, both the client and host will need an instance of the marshaller and parser. The idea of sharing this logic between both client and host ensures that one doesn't get out of sync with the other.
Putting it together, if you are writing a C++ client library, then you should be consuming the FirmataMarshaller
to form your messages to the host (Arduino) and the FirmataParser
in order to receive messages as callbacks from the host.
Thanks @zafields. I get it now :bulb:
@zafields I figured it out and fixed my library. Thanks again. If you are interested, here is my (yet another) implementation of the Firmata client library: https://github.com/dimitry-ishenko/firmata
@dimitry-ishenko I looked briefly at the syntax, I love it! I'll be checking it out again after work. Well done!
It looks to me as if extended analog only covers analog write (PWM). How do you query or
REPORT_ANALOG
on pins greater than 15?Also, the
PIN_TO_ANALOG(p)
macro is architecture dependent and not portable. It would be nice to have a generic way for it to do its job.On the other hand, much of this complexity could be eliminated altogether if there were an extended analog reporting scheme and
0xC0
was deprecated in its favor.