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

The analog conundrum #75

Open
zfields opened this issue Dec 8, 2016 · 25 comments
Open

The analog conundrum #75

zfields opened this issue Dec 8, 2016 · 25 comments
Labels

Comments

@zfields
Copy link
Contributor

zfields commented Dec 8, 2016

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.

@zfields
Copy link
Contributor Author

zfields commented Dec 8, 2016

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

@zfields
Copy link
Contributor Author

zfields commented Dec 8, 2016

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

@soundanalogous
Copy link
Member

soundanalogous commented Dec 8, 2016

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.

@soundanalogous
Copy link
Member

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.

@soundanalogous
Copy link
Member

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));
    }
  }
}

@soundanalogous
Copy link
Member

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

@soundanalogous
Copy link
Member

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

@soundanalogous
Copy link
Member

soundanalogous commented Dec 10, 2016

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

@soundanalogous
Copy link
Member

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

@zfields
Copy link
Contributor Author

zfields commented Dec 12, 2016

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.

@soundanalogous
Copy link
Member

soundanalogous commented Dec 12, 2016

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?

@soundanalogous
Copy link
Member

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.

@zfields
Copy link
Contributor Author

zfields commented Dec 14, 2016

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

@soundanalogous
Copy link
Member

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

@dimitry-ishenko
Copy link

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.

@zafields
Copy link

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

@dimitry-ishenko
Copy link

Thanks @zafields. I took a peek, but... no cigar 😄

@zafields
Copy link

zafields commented Sep 18, 2017

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

@dimitry-ishenko
Copy link

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?

@zafields
Copy link

@dimitry-ishenko

This is a host side implementation of the protocol for Arduino, right?

No. It simply marshals calls into the Firmata protocol.

@dimitry-ishenko
Copy link

@zafields whose calls? Arduino's? So, is this a client side implementation for Arduino? Sorry, if I am asking stupid questions.

@zafields
Copy link

zafields commented Sep 19, 2017

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

@dimitry-ishenko
Copy link

Thanks @zafields. I get it now 💡

@dimitry-ishenko
Copy link

@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

@zfields
Copy link
Contributor Author

zfields commented Sep 19, 2017

@dimitry-ishenko I looked briefly at the syntax, I love it! I'll be checking it out again after work. Well done!

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

No branches or pull requests

4 participants