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() return value according to value set in analogReadResolution() #5776

Merged
merged 1 commit into from
Oct 24, 2021
Merged

analogRead() return value according to value set in analogReadResolution() #5776

merged 1 commit into from
Oct 24, 2021

Conversation

P-R-O-C-H-Y
Copy link
Member

Summary

Function analogReadResolution set how many bits will analogRead return.

Find out that this functionality was added back 2017 by @me-no-dev in #161.

Related issues:
#5163

Impact

No impact

@P-R-O-C-H-Y P-R-O-C-H-Y self-assigned this Oct 18, 2021
@@ -48,12 +48,26 @@ static uint8_t __analogVRefPin = 0;
static uint8_t __analogAttenuation = 3;//11db
#if CONFIG_IDF_TARGET_ESP32S2
static uint8_t __analogWidth = 4; // 13 bits
static uint8_t __analogReturnedWidth = 13;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can't we use some chip defines for those, instead of filtering by target?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, we should be able to use SOC_ADC_MAX_BITWIDTH for __analogReturnedWidth for all platforms. It would need to be tested if ADC_WIDTH_MAX value is consistent across supported platforms.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member Author

@P-R-O-C-H-Y P-R-O-C-H-Y Oct 22, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I made the changes according your sugestions :) tested with esp32 and esp32s2

@P-R-O-C-H-Y P-R-O-C-H-Y requested a review from me-no-dev October 22, 2021 15:01
@me-no-dev me-no-dev merged commit aabbed0 into espressif:master Oct 24, 2021
@me-no-dev
Copy link
Member

nice :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AnalogReadResolution and AnalogRead not handling low and high resolutions correctly
3 participants