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

[WIP] Esp32 I2S-based current sense (adc) driver #400

Draft
wants to merge 14 commits into
base: dev
Choose a base branch
from

Conversation

mcells
Copy link
Contributor

@mcells mcells commented May 19, 2024

Introduction

First, I am not certain if this is the right repo to put the code when it is ready for release, as it is quite HW-specific and has some constraints, and we already have a working current sense solution in place. Secondly, this is just referencing my developement branch, so the code has debugging options, some outdated comments and code, and is generally not very pretty.

Nonetheless, I want to get this out there for two reasons: On the one hand to possibly receive some opinions on the feasability of integrating this in the library at some point,
on the other hand, I feel this could be useful for some people, be it users of the library, or people working with the ESP32´s adc, looking for information.

On the driver

This driver uses the (original) ESP32´s continous adc sampling mode, where the I2S peripheral connects to the adc and clocks out samples at a configurable frequency without using cpu time.
This means that the adc sampling happens asynchronusly and non-blocking. Samples get pushed into the I2S´s 32-large FIFO automatically, where we only need to do a bit of bitshifting and save them.

This leads to the following usecases I implemented here:

  • Faster sampling than the current implementation (here a fixed 1MS/s vs. about 100KS/s for the current SFOC driver)
    • The individual samples may possibly have more noise this way, but I feel performance still improves due to the higher loop rates
  • Low-/Highside sensing with all phases sampled each PWM cycle, or even twice per cycle, versus sampling only one phase per PWM cycle due to time constraints
  • "Sampling in the past", or more specifically: Instead of starting aquisition when an interrupt occurs (eg, at Low-/Highside center), we stop sampling and read out the FIFO. This approximately centers the aquired samples in the center of the waveform and avoids artifacts at low-ish duty-cycles for Low-/Highside sampling (As well as Inline, when configuring as Low-/Highside).
  • We can do sensorless control using High Frequency Injection (HFI) at injection frequencies above human hearing range. This is part of the reason why I am developing this driver (apart from trying to more or less equal what is easily possible on STM32).

The driver works (for me at least) and does definitely improve performance. (Loop speed and signal quality)

There are however some caveats to all this:

  • This only works on the original ESP32, not on the ESP32Sx/-Hx/..., which offer other means to continously sample the adc. I don´t have any of the other ESP32 variants and am not sure if they can offer similarly high performance.
  • As is, this only runs on ADC1. Using both ADC1 and ADC2 lowers resolution from 12 to 11 Bit. Using ADC2 is not really that practical anyways, due to WIFI accessing it when activated.
  • You can´t use analogRead() on any of the ADC1 channels, as this will most probably break the setup, cause crashes, or just not work. I´ve not tried it though.
  • The code could just stop working any time.
    • Reason is, most stuff is Register level, some api calls are there as well. And especially the setting up of the adc and I2S is not really documented very well. (The IDF ESP32 source code is also many abstaction layers deep, which doesn´t help at all)
    • To get the latency and the processing time as low as possible required a lot of trial-and-error, as well as using effectively undocumented features, especially in the readfifo() method. It´s not so much voodoo, but more guesswork as to how stuff behaves in edge cases. (No help looking in the TRM, as many Registers are just mentioned and not explained)

@runger1101001 runger1101001 added the enhancement New feature or request label May 20, 2024
@runger1101001
Copy link
Member

Hi @mcells, thanks a lot for this contribution and all the effort!

I see its still just a draft, but I don't know if we'll be able to merge it that soon...

  • at the moment it is making many test cases fail, not just ESP32 related ones. We'd definately have to get the code separated out in a way that it affects only ESP32 and no other architectures, and within ESP32 only the supported chips.

  • while we have a structure for this (for example in stm32) I have to say that we're discussing how to improve this in the future - it turns out current sensing is both more complicated, and has more optionality than PWM, so I think we would better capture the needs with an abstraction more like the sensors, based on classes, where users can "plug in" different types of current sensing at compile time and the whole process is more under user control.

  • so I would actually see this more in that future, as a "ESP32CurrentSenseI2SDriver" available to those that want/need it

  • and because it supports only a subset of ESP32 and has specific advantages for certain use cases like HFI, and also because you say "The code could just stop working any time." I would personally see it as part of the SimpleFOC Drivers repository, where we put code that is hardware specific and/or less supported than the main library.

  • another point to make is that ESP32 IDF 3.0 for Arduino is around the corner, with alpha releases already available and required for ESP32 C6 for example, and this new version of the framework brings many incompatible API changes... so we're about to have to upgrade all the ESP32 code in a major way, and its not the optimal time to extend the codebase in complex ways. So also from this point of view I would like to wait a bit before we attempt to merge anything anywhere...

I don't want to sound discouraging, the effort and work is really appreciated! We have a subset of users very interested in HFI, as you may know, and so this will indeed be interesting to people...

What do you say? Is it ok if we take it slowly on this, wait for ESP32 IDF 3.0 to come out in final release, and then work towards putting into SimpleFOC 3.0 with a better current sense driver architecture and possibly a API for HFI stuff?

@Copper280z
Copy link
Contributor

Copper280z commented May 21, 2024

Seems the test failures are mostly related to adding the conversion function call in the inline class, while the other platforms don't have this function defined.
mcells@b68fedf

PhaseCurrent_s InlineCurrentSense::getPhaseCurrents(){
    PhaseCurrent_s current;
    _startADC3PinConversionInline();  // <<<<<<
    current.a = (!_isset(pinA)) ? 0 : (_readADCVoltageInline(pinA, params) - offset_ia)*gain_a;// amps
    current.b = (!_isset(pinB)) ? 0 : (_readADCVoltageInline(pinB, params) - offset_ib)*gain_b;// amps
    current.c = (!_isset(pinC)) ? 0 : (_readADCVoltageInline(pinC, params) - offset_ic)*gain_c; // amps

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

Successfully merging this pull request may close these issues.

3 participants