JChristensen / JC_Button

Arduino library to debounce button switches, detect presses, releases, and long presses
GNU General Public License v3.0
425 stars 102 forks source link

library.properties says architecture is AVR only #21

Closed bperrybap closed 5 years ago

bperrybap commented 5 years ago

This is related to issues #4 and #17 in that the library properties says the library architecture is AVR only when it runs on other processors. The reason I bring this up is that the IDE no longer shows examples for architectures/processors that are not in the library.properties architectures list.

The button code doesn't use anything outside of the standard Arduino core functions so it should work on any Arduino core. Using a star "*" rather than "avr" would allow the examples to show up on all the cores and remove a warning that the IDE will issue when using a library on an architecture not specified in the library.properties i.e. in the case of using it on a ESP8266 processor you will not see or be able to access any of the examples and you will get this warning when using it your sketches:

WARNING: library JC_Button claims to run on (avr) architecture(s) and may be incompatible with your current board which runs on (esp8266) architecture(s).
JChristensen commented 5 years ago

I will give the matter some additional consideration but that seems like a totally inappropriate assumption on the part of the IDE.

bperrybap commented 5 years ago

What they've done seems to make some sense to me. In their view the IDE is told how to behave by what the author specifies in the library.properties file. The IDE filters out examples from libraries whose library.properties file does not claim to work on the core associated with the selected board type. What surprises me more is that the Arduino builder will still find libraries and allow them to be used on cores that the library doesn't say it works on.

There was lots of discussion about this on the mailing list a while back. The issue was/is that there are quite a few libraries that do CPU specific things rather than stick to Arduino core functions so they will only run on a certain processor, like doing raw AVR port i/o. People were getting all kinds of funky errors when trying to use those libraries on the newer boards like the DUE. So to make it easier for users and to try to keep the library authors and forum from getting threads about the same types of questions over and over again from things like AVR code blowing up on non AVR cores, the IDE now allows a library author to specify which cores/architectures work with the library. It is the choice of the library author which cores can "see" the library.

JChristensen commented 5 years ago

I just tested with board set to DUE, and the library is visible. I can ask the IDE to include the library via Sketch > Include Library. I can compile and then I see the warning that the library may be incompatible. I'm fine with all that.

However, the examples are not visible. That is inconsistent, and IMHO wrong. It's a bad assumption that just because I am using a certain architecture that I will never be interested in examples for a different architecture.

davidmpye commented 5 years ago

As another user of JC_button, is there a particular reason you want to specify an architecture here?

There doesn't seem to be anything AVR specific about it, and I'm using it (and it works great btw!) on ESP8266 :-)

bperrybap commented 5 years ago

@JChristensen I agree with you about the inconsistency. However, I agree with @davidmpye For a library like this, that uses nothing but standard Arduino core functions, it should work on all architectures, so it would not seem necessary to specify that it is for the avr architecture.

Looking back in history, if I remember correctly, the Arduino development team went the other way in that I think there was at least one version of the IDE a while back where it was fully consistent but rather than show the examples for the other architectures, not only would the examples not show up but the library would not be available to other cores. i.e. it was only available to the specified architectures.

Also, I seem to recall that they didn't initially support a wildcard architecture - but that may have been in the early discussions vs an actual IDE release. I remember Paul and I arguing for a wild card architecture. But they didn't want it since they didn't want code to use ifdefs for architectures. That was back in the day where the Arduino team was pushing a libraries directory structure where each architecture would have its own src sub directory in the same library directory and Paul and I were pushing to allow each core to have its own libraries directory since their way was completely unmanageable. They didn't see the light until they released the DUE core. Then they ended up doing what Paul and I were asking them to do for the previous two years.

davidmpye commented 5 years ago

It's also worth pointing out that other IDEs (eg https://platformio.org/lib/show/77/JC_Button) also restrict the library's availability based on platform :-/

JChristensen commented 5 years ago

@bperrybap Ha, I missed all of that, probably just as well ;-) @davidmpye I only use the AVR architecture myself so that's what I've been indicating in my libraries. I do not test them on other architectures nor am I in a position to debug any issues that may arise on other architectures. I do not necessarily intend to take ownership of any compatibility issues with other architectures. I understand that this particular library has generic code and so should work on any architecture, but I can't quite bring myself to bet 100% on that. I think that the user deserves to know that the library was developed on AVR and only tested on AVR. If it works on another architecture, then that is all well and good but if it were me, I'd want to know exactly where I stand. If the user takes issue with the warning message, then it's a trivial thing for them to update one line in their library.properties file.