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

Updated MCU HAL to support other chips in the ESP32 family #649

Merged
merged 2 commits into from
Feb 28, 2024

Conversation

lonelycorn
Copy link
Contributor

in this pull request, the MCU HALL to support other chips in the ESP32 family which have different capabilities than ESP32:

  • not all of them support LEDC_HIGH_SPEED_MODE: PWM speed mode is additionally guarded by SOC_LEDC_SUPPORT_HS_MODE
  • some has fixed ADC resolution (e.g. 12-bit): ADC is configured to run with max resolution, and the results are converted to 10-bit using bitshift

@Paciente8159
Copy link
Owner

Awesome job. Thanks.

Just one question regarding ADC. Wouldn't be preferable to simply set the ADC to ADC_WIDTH_BIT_12
and then bit-shift 2 bits?
The SDK seems to indicate the higher bit-width ADC are available.

    ADC_WIDTH_BIT_9  = 0, /*!< ADC capture width is 9Bit. */
    ADC_WIDTH_BIT_10 = 1, /*!< ADC capture width is 10Bit. */
    ADC_WIDTH_BIT_11 = 2, /*!< ADC capture width is 11Bit. */
    ADC_WIDTH_BIT_12 = 3, /*!< ADC capture width is 12Bit. */
#elif SOC_ADC_MAX_BITWIDTH == 12
    ADC_WIDTH_BIT_12 = 3, /*!< ADC capture width is 12Bit. */
#elif SOC_ADC_MAX_BITWIDTH == 13
    ADC_WIDTH_BIT_13 = 4, /*!< ADC capture width is 13Bit. */
#endif
    ADC_WIDTH_MAX,

This is transversal to ESP32, ESP32C3, ESP32S2 and ESP32S3. They all seem to support the ADC_WIDTH_BIT_12 definition. Are newer variants that have other definitions?

@Paciente8159 Paciente8159 merged commit d980950 into Paciente8159:master Feb 28, 2024
27 checks passed
@lonelycorn
Copy link
Contributor Author

lonelycorn commented Feb 28, 2024

@Paciente8159 I was just getting started with ESP32 family and playing with a ESP32S3 dev board which only supports 12-bit ADC. I also read that "10bit ADC" is sort of a "breaking change" introduced in v1.8. So I decided to keep the output of mcu_get_analog() in the same range of [0, 4095] just in case.

I did not verify if ADC_WIDTH_BIT_12 was available on all current MCU's of this family. That said, the logic implemented in this PR should work regardless, which IMO is a bit more future-proof?

@Paciente8159
Copy link
Owner

Yes. I applied the same logic to attenuation too. Thanks

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.

None yet

2 participants