Skip to content

Conversation

@mennovf
Copy link
Contributor

@mennovf mennovf commented Apr 9, 2025

Thank you for your contribution!

We appreciate the time and effort you've put into this pull request.
To help us review it efficiently, please ensure you've gone through the following checklist:

Submission Checklist 📝

  • I have updated existing examples or added new ones (if applicable).
  • I have used cargo xtask fmt-packages command to ensure that all changed code is formatted correctly.
  • My changes were added to the CHANGELOG.md in the proper section.
  • I have added necessary changes to user code to the Migration Guide.
  • My changes are in accordance to the esp-rs developer guidelines

Extra:

Pull Request Details 📖

I'm working on implementing ADC calibration for the ESP32-S2. I've made some progress, specifically reading calibration values from eFuse. I followed the code path in IDF’s examples/peripherals/adc/oneshot_read/main/oneshot_read_main.c as a guide.

Still running into a few issues:

  • init_code:
    The V1 eFuse lacks an init_code, so manual calibration is required, but I haven’t gotten it to work yet.

    • set_init_code didn’t do anything until @JurajSadel’s recent regi2c commits for the S2.

    • Right now, the manual calibration logic lives in AdcConfig::enable_pin_with_cal(). But this runs before the ADC peripheral is initialized. This means it will hang on while !ADCI::is_done() {} and just not work properly in general. Probably better to add a calibrate() method to AdcCalSource and call it from Adc::new() in a loop over the pins.

    • Manual calibration always reads 0 for me, even though post-calibration reads do register a signal. I tested this by copy-pasting the Adc::new init code into adc_calibrate to get past the init ordering issue. The idf's self-calibration init code is around 0x680.

  • scheme_line_fitting from IDF:
    This calculates slope and y-intercept from eFuse or a self-cal step. The y-intercept is often positive, which makes it unusable with AdcCalBasic, since that applies correction before the scaling in AdcCalLine.
    Also, the example seems to use both init_code AND a full affine correction

  • Cleanup

Description

Implement adc calibration for the esp32s2.

#1203
#326

Testing

Some manual testing, not yet finished.
I get the same efuse line calibration values as idf.

@bugadani
Copy link
Contributor

bugadani commented Apr 9, 2025

A few upfront "don't forget me"s:

  • please don't reformat Cargo.toml
  • please don't check in your rust-analyzer config
  • please don't create a completely new way of dealing with efuses. We should have the existing bits under soc, generated from the relevant csv I believe.
  • please no unconditional defmt::println logs. You should use one of the macros from fmt.rs

@mennovf
Copy link
Contributor Author

mennovf commented Apr 9, 2025

Yes, I will remove and squash all of this eventually, but thanks for pointing them out.
I posted this as a draft for guidance on how to proceed. I will have a look at the soc efuse bit.

@mennovf
Copy link
Contributor Author

mennovf commented Apr 9, 2025

  • please don't create a completely new way of dealing with efuses. We should have the existing bits under soc, generated from the relevant csv I believe.

The esp32s2 somewhat uniquely has this weird adc_efuse_raw_map. Would it be fine to keep the table, but replace the const TAG_ efuse field and MapInfo::block/begin_bit/length duplication with a reference to the efuse/fields.rs? There's still the issue that idf's signed_bit_to_int function needs access to the private EfuseField::bit_len.

@MabezDev
Copy link
Member

MabezDev commented May 1, 2025

This probably needs to be rebased on top of #3440 once its merged

@mennovf mennovf force-pushed the feature-esp32s2-adccal branch from 3cda292 to 91245ce Compare July 18, 2025 11:33
@mennovf
Copy link
Contributor Author

mennovf commented Jul 18, 2025

I have put more time into figuring this out and have used a different board. I am now using the V2 version of the efuse where a line calibration is embedded, instead of having to do a manual calibration (which I will now abandon).
With just the AdcCalBasic I already get some decent raw values, but they differ from the idf/adc_oneshot example. I have spent hours pouring over the code, trying/adding idf stuff but could not get it to the correct raw value.
For a ~1.65V reference I measure ~4600 (1.5V) whereas the idf example measures ~5300 (1.65V). After conversion to mV, the idf example is correct.
I don't think there's much going on here with the CalBasic to affect this result, just setting the init_code. But the init_code is the same for both versions, even when inspecting the final command words that are written to the internal I2C peripheral to set the (msb, lsb) registers of the SAR. Manually changing which init_code gets sent, does affect the raw value that is read.

In short: I am stuck.

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.

3 participants