fix analogReference() - #643
Open
wke67 wants to merge 1 commit into
Open
Conversation
…NAL.. defines. Checking of mode bits is already done by check_valid_analog_ref(), no need to check twice.
k-takata
reviewed
Sep 7, 2026
Comment on lines
+942
to
+943
| check_valid_analog_ref(mode); // does all the checking | ||
| VREF.ADC0REF = (VREF.ADC0REF & ~(VREF_REFSEL_gm))|(mode & (VREF_REFSEL_gm)); |
There was a problem hiding this comment.
It seems check_valid_analog_ref() checks only at compile time.
Isn't it better to keep runtime checking? E.g.:
Suggested change
| check_valid_analog_ref(mode); // does all the checking | |
| VREF.ADC0REF = (VREF.ADC0REF & ~(VREF_REFSEL_gm))|(mode & (VREF_REFSEL_gm)); | |
| check_valid_analog_ref(mode); | |
| mode &= VREF_REFSEL_gm; | |
| if (mode < 7 && mode != 4) { | |
| VREF.ADC0REF = (VREF.ADC0REF & ~(VREF_REFSEL_gm))|(mode); | |
| } |
Contributor
Author
There was a problem hiding this comment.
I agree, runtime checking is a good thing - but only if you can return an error code, which you can not do here.
The runtime checking is done partially by masking the mode value with VREF_REFSEL_gm, I think that is good enough. BTW, the hard part in debugging this error was to understand why the function did not modify the register, only the source code could tell.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
this fixes analogReference(), issues #639 #626.
analogReference() did not reflect the last changes to INTERNAL2V5 etc. defines. Checking the mode bits is already done by check_valid_analog_ref(), no need to check twice!