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

Add ADC support STM32L4xx and STM32G4xx series MCUs #22341

Merged
merged 12 commits into from
Dec 8, 2023

Conversation

Cipulot
Copy link
Contributor

@Cipulot Cipulot commented Oct 26, 2023

Description

These changes are based on @sigprof branch that he mentioned in a comment under #19922.

I've included his original changes that handle the dummy conversion to avoid ADC errors and also implemented the pinToMux() for both L4 and G4 series.

Implemented a check for the default ADC_SAMPLING_RATE, defaulting to ADC_SMPR_SMP_2P5 since it's the fastest available as per 'hal_adc_lld.h' in ChibiOS and datasheets for the L4 and G4.

I removed the debug print he included since it's not needed.

Tested with G431CBU, and confirmed working with pins of ADC1 and ADC2. Appreciate if others will test on some of the L4 MCUs supported in QMK, since I have no access to them as of now.

Change of the documentation about ADC support in the STM series will follow when this approved.

Types of Changes

  • Core
  • Bugfix
  • New feature
  • Enhancement/optimization
  • Keyboard (addition or update)
  • Keymap/layout/userspace (addition or update)
  • Documentation

Issues Fixed or Closed by This PR

Checklist

  • My code follows the code style of this project: C, Python
  • I have read the PR Checklist document and have made the appropriate changes.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • I have tested the changes and verified that they work and don't break anything (as well as I can manage).

@github-actions github-actions bot added the core label Oct 26, 2023
Copy link
Contributor

@sigprof sigprof left a comment

Choose a reason for hiding this comment

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

Looks like this was based just on the first commit (8435456), and not the whole branch (master...sigprof:qmk_firmware:adc-stm32l4xx)? At least the RP2040 breakage seems to be still there.

platforms/chibios/drivers/analog.c Outdated Show resolved Hide resolved
platforms/chibios/drivers/analog.c Outdated Show resolved Hide resolved
@sigprof
Copy link
Contributor

sigprof commented Oct 28, 2023

As for the sampling rate, the problem is that the fastest sampling also requires the lowest impedance of the signal source, and the required impedance is much lower than what AVR (or even RP2040) would require. E.g., if you look at the G431 datasheet:
STM32G431 datasheet, page 121
At 2.5 cycles there is even no valid value for the source impedance if a “slow” channel is used (anything except ADCx_IN1…ADCx_IN5).

One typical usage of ADC is to connect a potentiometer-based joystick, and the typical resistance of those potentiometers is 10 kΩ; this obviously needs a much higher sampling time than 2.5 cycles, otherwise the readings are significantly off. So one option is to set a value which would be appropriate for such source as the default; but the problem is that it would also depend on the ADC clock frequency, and there does not seem to be a simple way to take that into account (in theory different ADC might also have different clock frequencies). Another option is just to set the fastest value (as was done for other STM32 chips, even though the default ADC clocks for them can be vastly different) and hope that any joystick users would understand that they need to change that value to something slower to get proper readings.

@Cipulot
Copy link
Contributor Author

Cipulot commented Oct 28, 2023

I was actually unsure about the sampling rate situation myself tbh. An idea that I had was to check if features like joystick etc are enabled, and if so to print a message in the terminal. Adding a note in the documentation once I update that is gonna be helpful too, both in the joystick section and in the ADC specs.

@github-actions github-actions bot added keyboard keymap via Adds via keymap and/or updates keyboard for via support dd Data Driven Changes labels Oct 28, 2023
@Cipulot
Copy link
Contributor Author

Cipulot commented Oct 28, 2023

Also @sigprof did you test the support for oversampling for ADCv3? I saw that you had something in a branch of yours but I didn't include it in this PR. If by your testing that's good I'll amend this PR to include that too.

@Cipulot
Copy link
Contributor Author

Cipulot commented Nov 10, 2023

It seems like the CI Build fails are all using RP2040 ADC. Errors arise in the ADC_SAMPLING_Rate #ifdef.... will investigate in order to fix this.

Fix for RP2040 build errors
…com/Cipulot/qmk_firmware into adc-add-stm32l4xx-stm32g4xx"

This reverts commit b11c297, reversing
changes made to ed3051f.
@github-actions github-actions bot removed keyboard keymap via Adds via keymap and/or updates keyboard for via support dd Data Driven Changes labels Nov 11, 2023
Attempt fix for formatting CI error
@drashna drashna requested a review from a team November 23, 2023 23:06
@tzarc tzarc requested review from sigprof and a team November 28, 2023 06:52
platforms/chibios/drivers/analog.c Outdated Show resolved Hide resolved
platforms/chibios/drivers/analog.c Outdated Show resolved Hide resolved
platforms/chibios/drivers/analog.c Outdated Show resolved Hide resolved
@zvecr zvecr merged commit 81cedf5 into qmk:develop Dec 8, 2023
4 checks passed
@Cipulot Cipulot deleted the adc-add-stm32l4xx-stm32g4xx branch December 8, 2023 01:29
itsjonny96 pushed a commit to itsjonny96/qmk_firmware that referenced this pull request Jan 7, 2024
* Update analog.c

* Changes to remove errors in compile

* Update analog.c

Fix for RP2040 build errors

* Revert "Merge branch 'adc-add-stm32l4xx-stm32g4xx' of https://github.com/Cipulot/qmk_firmware into adc-add-stm32l4xx-stm32g4xx"

This reverts commit b11c297, reversing
changes made to ed3051f.

* Update analog.c

Attempt fix for formatting CI error

* Update platforms/chibios/drivers/analog.c

Co-authored-by: Joel Challis <[email protected]>

* Update platforms/chibios/drivers/analog.c

Co-authored-by: Joel Challis <[email protected]>

* Update platforms/chibios/drivers/analog.c

Co-authored-by: Joel Challis <[email protected]>

---------

Co-authored-by: Joel Challis <[email protected]>
mechlovin pushed a commit to mechlovin/qmk_firmware that referenced this pull request Jan 17, 2024
* Update analog.c

* Changes to remove errors in compile

* Update analog.c

Fix for RP2040 build errors

* Revert "Merge branch 'adc-add-stm32l4xx-stm32g4xx' of https://github.com/Cipulot/qmk_firmware into adc-add-stm32l4xx-stm32g4xx"

This reverts commit b11c297, reversing
changes made to ed3051f.

* Update analog.c

Attempt fix for formatting CI error

* Update platforms/chibios/drivers/analog.c

Co-authored-by: Joel Challis <[email protected]>

* Update platforms/chibios/drivers/analog.c

Co-authored-by: Joel Challis <[email protected]>

* Update platforms/chibios/drivers/analog.c

Co-authored-by: Joel Challis <[email protected]>

---------

Co-authored-by: Joel Challis <[email protected]>
mechlovin pushed a commit to mechlovin/qmk_firmware that referenced this pull request Jan 19, 2024
* Update analog.c

* Changes to remove errors in compile

* Update analog.c

Fix for RP2040 build errors

* Revert "Merge branch 'adc-add-stm32l4xx-stm32g4xx' of https://github.com/Cipulot/qmk_firmware into adc-add-stm32l4xx-stm32g4xx"

This reverts commit b11c297, reversing
changes made to ed3051f.

* Update analog.c

Attempt fix for formatting CI error

* Update platforms/chibios/drivers/analog.c

Co-authored-by: Joel Challis <[email protected]>

* Update platforms/chibios/drivers/analog.c

Co-authored-by: Joel Challis <[email protected]>

* Update platforms/chibios/drivers/analog.c

Co-authored-by: Joel Challis <[email protected]>

---------

Co-authored-by: Joel Challis <[email protected]>
nuess0r pushed a commit to nuess0r/qmk_firmware that referenced this pull request Sep 8, 2024
* Update analog.c

* Changes to remove errors in compile

* Update analog.c

Fix for RP2040 build errors

* Revert "Merge branch 'adc-add-stm32l4xx-stm32g4xx' of https://github.com/Cipulot/qmk_firmware into adc-add-stm32l4xx-stm32g4xx"

This reverts commit b11c297, reversing
changes made to ed3051f.

* Update analog.c

Attempt fix for formatting CI error

* Update platforms/chibios/drivers/analog.c

Co-authored-by: Joel Challis <[email protected]>

* Update platforms/chibios/drivers/analog.c

Co-authored-by: Joel Challis <[email protected]>

* Update platforms/chibios/drivers/analog.c

Co-authored-by: Joel Challis <[email protected]>

---------

Co-authored-by: Joel Challis <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants