Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Move analogReference enum to ArduinoCore-API #236

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

Closed
rubenstar wants to merge 1 commit into arduino:master from rubenstar:master
Closed

Move analogReference enum to ArduinoCore-API #236

rubenstar wants to merge 1 commit into arduino:master from rubenstar:master

Conversation

Copy link

@rubenstar rubenstar commented Jun 14, 2024 •
edited
Loading

Currently, the header for analogReference(...) is in ArduinoCore-API and takes uint8_t as argument type.

The implementation of that function is in ArduinoCore-samd (here), but takes eAnalogReference enum as argument type. The enum is defined in Arduino.h

Compiling ArduinoCore-samd/cores/arduino/wiring_analog.c will give a warning:

[ 31%] Building C object .../ArduinoCore-samd/cores/arduino/wiring_analog.c.obj
/home/.../ArduinoCore-samd/cores/arduino/wiring_analog.c:94:6: warning: conflicting types for 'analogReference' due to enum/integer mismatch; have 'void(eAnalogReference)' {aka 'void(enum _eAnalogReference)'} [-Wenum-int-mismatch]
 94 | void analogReference(eAnalogReference mode)
 | ^~~~~~~~~~~~~~~
In file included from /home/.../ArduinoCore-API/api/ArduinoAPI.h:50,
 from /home/.../ArduinoCore-samd/cores/arduino/Arduino.h:23,
 from /home/.../ArduinoCore-samd/cores/arduino/wiring_analog.c:19:
/home/.../ArduinoCore-API/api/Common.h:100:6: note: previous declaration of 'analogReference' with type 'void(uint8_t)' {aka 'void(unsigned char)'}
 100 | void analogReference(uint8_t mode);
 | ^~~~~~~~~~~~~~~

My proposal is to move that enum from Arduino.h in ArduinoCore-samd fully into ArduinoCore-API. Seems cleaner to me.

I filed a corresponding PR in ArduinoCore-samd too:
arduino/ArduinoCore-samd#718

Copy link

CLAassistant commented Jun 14, 2024 •
edited
Loading

CLA assistant check
All committers have signed the CLA.

Copy link

bmourit commented Jun 28, 2024 •
edited
Loading

These values seem very specific. How can other platforms use them? Does it benefit any platform besides SAMD? Just curious as to if this is generic enough for other platforms following the API.

Copy link
Member

Hi @rubenstar, as @bmourit already explained, the analogReference parameters are architecture specific, so they cannot be generalized and added to the API repo.
The warning arises from the SAMD port being stricter than usual on the parameter type, but the fix for the warning should just be applied in that core.

Copy link
Author

@facchinm @bmourit Thanks for your input, that makes perfect sense. Sorry for raising the PR before really thinking it through.
facchinm and bmourit reacted with heart emoji

Copy link

bmourit commented Jul 1, 2024

No need to be sorry!!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /