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

Dependency problem on AVR specific #include. #251

Open
RobTillaart opened this issue Dec 20, 2020 · 10 comments
Open

Dependency problem on AVR specific #include. #251

RobTillaart opened this issue Dec 20, 2020 · 10 comments
Labels
arduino mocks Compilation mocks for the Arduino library enhancement New feature or request not yet Awaits better environmental conditions some archs Only affects some Arduino architectures

Comments

@RobTillaart
Copy link

Hi,

am struggling with a library - https://github.com/RobTillaart/DS18B20_INT

The library needs the "OneWire: so I adapted .arduino-ci.yml

compile:
  # Choosing to run compilation tests on 2 different Arduino platforms
  platforms:
    - uno
    - leonardo
    - due
    - zero
  # Declaring Dependent Arduino Libraries (to be installed via the Arduino Library Manager)
  libraries:
    - "OneWire"

unittest:
  # These dependent libraries will be installed
  libraries:
    - "OneWire"
    - "util/crc16"   <<<<<<<<<<<<<<<<<<< does not work.

The example of the lib now compiles correctly, however the unit test fails on util/crc16.h  which is used by the OneWire lib.
The path (on my PC) is   ...\Arduino\hardware\tools\avr\avr\include\util\crc16.h  but I do not know how to include it in the unit test.

It might be a feature that need to be solved in the future.

For now I will disable the unit test.

@ianfixes
Copy link
Collaborator

Can you post the output from the unit test run? The libraries that you list in the config file refer to the names that the library manager would use, not paths on disk. If utl/crc16.h isn't being made available to the unit tests, that's the real bug here.

@RobTillaart
Copy link
Author

LOG_DS28CM00_LIB.zip

This log is from the DS28CM00 library [this is an ID chip] - https://github.com/RobTillaart/DS28CM00/tree/arduino-ci

  • The unit test compiles and runs as expected.
  • Three examples
  1. Compiles as expected
  2. fails on #include util/crc16.h
  3. fails on #include rom/crc.h This example is an ESP32 specific example of previous.

The problem 3 is expected and need to be fixed. I will merge [2] and [3] and use conditional compilation.
When I get ESP32 to work it will be tested too. (another story).

@ianfixes ianfixes added this to the 2020 Wrapup milestone Dec 23, 2020
@ianfixes
Copy link
Collaborator

@per1234 does the use of utli/crc16.h or rom/crc.h fall under either "undocumented API" or "platform-specific feature"?

I'm tempted to close this as "works as intended", with limiting the set of test platforms & adding preprocessor directives being the proper remedy. But I want to double check that assumption.

@ianfixes ianfixes added the some archs Only affects some Arduino architectures label Dec 28, 2020
@per1234
Copy link
Contributor

per1234 commented Dec 29, 2020

"undocumented API"

util/crc16.h

it's definitely documented: https://www.nongnu.org/avr-libc/user-manual/group__util__crc.html

rom/crc.h

I'm not sure. I see there is doxygen documentation in the source code:
https://github.com/espressif/esp-idf/blob/v3.3.4/components/esp32/include/rom/crc.h
but I didn't find that documentation published anywhere. I really don't have much experience with the ESP32.

"platform-specific feature"

It looks like both of these are platform-specific.

@ianfixes
Copy link
Collaborator

Hmm. I'll probably deprioritize this then, sorry Rob.

My reasoning here is that it seems fruitless to try and replicate all functions that might exist in any platform library (vs existing in the "core" set of Arduino library functions). I had enough trouble with the AVR side of things, and don't want to go further down that path unless/until I can find a smarter way to do it.

I can revisit this if there is movement on arduino/arduino-cli#1090 or arduino/arduino-cli#1092

@ianfixes ianfixes added arduino mocks Compilation mocks for the Arduino library enhancement New feature or request not yet Awaits better environmental conditions labels Dec 29, 2020
@RobTillaart
Copy link
Author

RobTillaart commented Dec 29, 2020

Hmm. I'll probably deprioritize this then, sorry Rob.

OK, but as the OneWire library is used for several devices / libraries, all these will be affected.
So please do not close.

If I find a workaround I will let you know.

@ianfixes
Copy link
Collaborator

I won't close it. It's definitely something that needs to be addressed and the good news is that there are a few ways to address it. I just have to figure out which of the possible workarounds is the appropriate one and/or whether the approach in OneWire is (or isn't) considered a best practice.

@ianfixes
Copy link
Collaborator

ianfixes commented Jan 11, 2021

@per1234 regarding "undocumented API", would you say that something like portModeRegister(P) is undocumented? I see it listed here:
https://github.com/arduino/ArduinoCore-avr/blob/master/cores/arduino/Arduino.h

But what assurance do I have that this macro will always be provided by the core? My brief google search for anything in the online arduino reference documentation was unsuccessful.

@per1234
Copy link
Contributor

per1234 commented Jan 11, 2021

This is definitely a problem. It's difficult to say whether some of these things are undocumented because nobody ever got around to documenting them (or even creating a place to document them, since this is not something you would want to expose a beginner to), or if it was only ever intended for internal use by the core library.

There is a request for a more comprehensive documentation of the platform interface here: arduino/arduino-cli#985
There is also an interesting related discussion about breakage the transition to using ArduinoCore-API in ArduinoCore-samd caused to some code that relied on the undocumented interfaces: arduino/ArduinoCore-samd#587

@RobTillaart
Copy link
Author

It's difficult to say whether some of these things are undocumented because nobody ever got around to documenting them

Arduino was intended to learn people to do fun stuff, this level of coding was explicitly the level they wanted to hide, not only for beginners but for everyone.

For the libraries / unit test, I can live perfectly with some things that won't work in a unit test. Should not be too much of course, but when developing I often depend on testing with real hardware (sensor etc). The unit tests are most valuable for PR's fro 3rd parties to check they didn't break too much.

For me the compilation of examples are the top priority, as these are the starting points for the users of the libs.

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 enhancement New feature or request not yet Awaits better environmental conditions some archs Only affects some Arduino architectures
Projects
None yet
Development

No branches or pull requests

3 participants