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

AnalogRead() does not accept A0 as pin number #242

Open
RobTillaart opened this issue Dec 9, 2020 · 9 comments
Open

AnalogRead() does not accept A0 as pin number #242

RobTillaart opened this issue Dec 9, 2020 · 9 comments
Labels
arduino mocks Compilation mocks for the Arduino library bug Something isn't working

Comments

@RobTillaart
Copy link

In a unit test I use an analogRead(), however it will not accept A0 etc as typical pin names.
imho users expect these to work

@ianfixes ianfixes added ci scripts The test runner scripts enhancement New feature or request labels Dec 9, 2020
@ianfixes
Copy link
Collaborator

ianfixes commented Dec 9, 2020

@per1234 where should the A0 constants be defined? I'm not sure whether to tie them to individual boards, a family of boards, or a CPU architecture. My understanding is that the presence of the constants reflects the presence of the pins themselves -- not all boards will have the same set of pin constants. Is that accurate?

@ianfixes ianfixes added arduino mocks Compilation mocks for the Arduino library bug Something isn't working and removed ci scripts The test runner scripts enhancement New feature or request labels Dec 9, 2020
@per1234
Copy link
Contributor

per1234 commented Dec 9, 2020

where should the A0 constants be defined?

The common convention is in the core variant:
https://github.com/arduino/ArduinoCore-avr/blob/1.8.3/variants/standard/pins_arduino.h#L65

not all boards will have the same set of pin constants. Is that accurate?

Correct. You can see the Uno has A0-A7 defined, the Mega A0-A15, ESP8266 only A0, and so on.

@ianfixes
Copy link
Collaborator

ianfixes commented Dec 9, 2020

So the challenge for me will be to either figure out a way to put it (with proper #ifdef logic) into the board mocks, or else shoehorn it into the config file for board definitions.

Long term, the elephant in the room here is that I think I've only solved the hardware mocks/#define stuff for AVR, and not for any other platform. I'm not even sure how to rectify that.

@ianfixes
Copy link
Collaborator

ianfixes commented Dec 10, 2020

The common convention is in the core variant

I may be reaching the practical limits of what I can accomplish with hardware mocks. I don't have any sort of document (nor even a validator) for what macros might be defined across all of the hardware support and port registers, so there's no way to check that my mocks are in sync with those. And that ignores the problem of what might be included with any given "core install".

How should I be approaching this issue? @matthijskooijman based on your comments on #147 I'm really curious whether solving that is now the easiest path forward.

@RobTillaart
Copy link
Author

imho one could implement all A0..An for all boards to keep it simple and to get started.
I expect most users will not use "out of range" names in their code / test code, why should they?

@matthijskooijman
Copy link
Collaborator

To really properly mock different platforms and boards, I guess some separation along the same lines as the IDE uses (different platforms, and within platforms, different boards) would be the most powerful approach (i.e. have some AVR-specific stuff that is triggered by __AVR__ or so from the board config, similar for SAMD or ESP32, and then for each board some .h file and/or stuff in the board config, possibly even mimicing the board/variant separation as well). This can get complex quickly, though and doesn't scale well since it's quite a bit of work to get a single board completely right. But if you limit to just a few commonly-used boards, this might still work, though.

An alternative could be to do more extensive mocking of each platform, and then use the variants from the original platforms verbatim. This means that the (undocumented) interface from core to variant should be mocked as well, which will likely include low-level arch-specific stuff (e.g. AVR I/O registers, which you're already mocking now, but this might be more tricky for other architectures). This is more work for a platform, but then likely makes it a lot easier to support all boards of that platform (by copying variant files and board.txt contents, probably with some (automatable) transformations applied to them.

I'm not sure what the best approach is here. Also, my suggestion in #147 does not really help here, since if you make an "ArduinoCI" platform that arduino-cli knows how to compile, you'd still need to define all available boards and variants inside that platform. There's no way to "import" board definitions from other platforms, so you cannot magically get support for all other Arduino boards in this way somehow (and this can't really be implemented either, since board definitions and variants are currently heavily coupled to their platform).

imho one could implement all A0..An for all boards to keep it simple and to get started.
I expect most users will not use "out of range" names in their code / test code, why should they?

This also seems like a valid strategy. Essentially you then just do not try to perfectly mock each board with all its limitations, but instead you try to always mock enough so that most sketches will compile and run, even if the mocking is not perfect (i.e. because more pins are available than in reality).

@matthijskooijman
Copy link
Collaborator

So the challenge for me will be to either figure out a way to put it (with proper #ifdef logic) into the board mocks, [...]

Note that what you call "board mocks" (io*.h files and things like pgmspace.h) doesn't look like board mocks to me. That's AVR-specific and often MCU-specific header files that are normally provided by avr-libc (through the Arduino AVR core). They are MCU-specific, but not really board-specific. Also, I would recommend not putting extra stuff in those files, to keep them as verbatim as possible to simplify updates later. And adding board-specific stuff in there would conflate board-specific with MCU-specific, which probably only makes things harder...

@ianfixes
Copy link
Collaborator

I expect most users will not use "out of range" names in their code / test code, why should they?

They shouldn't. Since we allow testing compilation on multiple platforms, the appropriate errors should be raised during that step. It just feels wrong to me that the unit test compilation would allow something that couldn't be compiled in an actual sketch.

@ianfixes
Copy link
Collaborator

An alternative could be to do more extensive mocking of each platform, and then use the variants from the original platforms verbatim. This means that the (undocumented) interface from core to variant should be mocked as well, which will likely include low-level arch-specific stuff (e.g. AVR I/O registers, which you're already mocking now, but this might be more tricky for other architectures). This is more work for a platform, but then likely makes it a lot easier to support all boards of that platform (by copying variant files and board.txt contents, probably with some (automatable) transformations applied to them.

I'm interested in exploring this option first. Right now the way I'm copying the source files from Arduino's avr directory and hand-modifying them is too brittle for the long term.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
arduino mocks Compilation mocks for the Arduino library bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants