Move slow ADC to thread #630 (#1042)

* adc with thread

* enable

* check result

* stacktual embiggenment

* tracing

* let's be type safe

* improve assert

* format, comment

* remove EFI_INTERNAL_SLOW_ADC_PWM fully

firmware/config/boards/kinetis/efifeatures.h

@@ -264,7 +264,6 @@
 
 // todo: switch to continues ADC conversion for slow ADC?
 // https://github.com/rusefi/rusefi/issues/630
-#define EFI_INTERNAL_SLOW_ADC_PWM	&PWMD1
 // todo: switch to continues ADC conversion for fast ADC?
 #define EFI_INTERNAL_FAST_ADC_PWM	&PWMD2
 

firmware/config/boards/skeleton/efifeatures.h

@@ -267,7 +267,6 @@
 
 // todo: switch to continuous ADC conversion for slow ADC?
 // https://github.com/rusefi/rusefi/issues/630
-#define EFI_INTERNAL_SLOW_ADC_PWM	&PWMD8
 // todo: switch to continues ADC conversion for fast ADC?
 #define EFI_INTERNAL_FAST_ADC_PWM	&PWMD4
 

firmware/config/stm32f4ems/efifeatures.h

@@ -293,7 +293,6 @@
 
 // todo: switch to continues ADC conversion for slow ADC?
 // https://github.com/rusefi/rusefi/issues/630
-#define EFI_INTERNAL_SLOW_ADC_PWM	&PWMD8
 // todo: switch to continues ADC conversion for fast ADC?
 #define EFI_INTERNAL_FAST_ADC_PWM	&PWMD4
 

firmware/config/stm32f4ems/halconf.h

@@ -201,7 +201,7 @@
  * @note    Disabling this option saves both code and data space.
  */
 #if !defined(ADC_USE_MUTUAL_EXCLUSION) || defined(__DOXYGEN__)
-#define ADC_USE_MUTUAL_EXCLUSION    TRUE
+#define ADC_USE_MUTUAL_EXCLUSION    FALSE
 #endif
 
 /*===========================================================================*/

firmware/config/stm32f4ems/mcuconf.h

@@ -277,7 +277,7 @@
 #define STM32_PWM_USE_TIM4                  TRUE
 #define STM32_PWM_USE_TIM5                  FALSE
 // todo: https://github.com/rusefi/rusefi/issues/630 ?
-#define STM32_PWM_USE_TIM8                  TRUE
+#define STM32_PWM_USE_TIM8                  FALSE
 #define STM32_PWM_USE_TIM9                  FALSE
 #define STM32_PWM_TIM1_IRQ_PRIORITY         7
 #define STM32_PWM_TIM2_IRQ_PRIORITY         7

firmware/config/stm32f7ems/mcuconf.h

@@ -285,7 +285,7 @@
 #define STM32_PWM_USE_TIM3                  FALSE
 #define STM32_PWM_USE_TIM4                  TRUE
 #define STM32_PWM_USE_TIM5                  FALSE
-#define STM32_PWM_USE_TIM8                  TRUE
+#define STM32_PWM_USE_TIM8                  FALSE
 #define STM32_PWM_USE_TIM9                  FALSE
 #define STM32_PWM_TIM1_IRQ_PRIORITY         7
 #define STM32_PWM_TIM2_IRQ_PRIORITY         7

firmware/development/perf_trace.h

@@ -30,7 +30,7 @@ enum class PE : uint8_t {
 	PeriodicControllerPeriodicTask,
 	PeriodicTimerControllerPeriodicTask,
 	AdcCallbackFast,
-	AdcCallbackSlow,
+	AdcProcessSlow,
 	AdcConversionSlow,
 	AdcConversionFast,
 	AdcSubscriptionUpdateSubscribers,

firmware/hw_layer/AdcConfiguration.h

@@ -37,7 +37,7 @@ public:
 	// F4 does not care
 	__ALIGNED(32) adcsample_t samples[ADC_MAX_CHANNELS_COUNT * MAX_ADC_GRP_BUF_DEPTH];
 	// Assert multiple of 32 bytes long so we don't stomp on the data after the buffer
-	static_assert(sizeof(samples) % 32 == 0, "ADC sample buffer alignment");
+	static_assert(sizeof(samples) % 32 == 0, "ADC sample buffer size must be a multiple of 32 bytes");
 
 	int getAdcValueByHwChannel(int hwChannel) const;
 

firmware/hw_layer/adc_inputs.cpp

@@ -30,6 +30,7 @@
 #include "adc_subscription.h"
 #include "AdcConfiguration.h"
 #include "mpu_util.h"
+#include "periodic_thread_controller.h"
 
 #include "pin_repository.h"
 #include "engine_math.h"
@@ -71,14 +72,6 @@ AdcDevice::AdcDevice(ADCConversionGroup* hwConfig) {
 	memset(internalAdcIndexByHardwareIndex, 0xFFFFFFFF, sizeof(internalAdcIndexByHardwareIndex));
 }
 
-#if !defined(PWM_FREQ_SLOW) || !defined(PWM_PERIOD_SLOW)
-// todo: migrate from hardware timer to software ADC conversion triggering
-// todo: I guess we would have to use ChibiOS timer and not our own timer because
-// todo: adcStartConversionI requires OS lock. currently slow ADC is 20Hz
-#define PWM_FREQ_SLOW 5000   /* PWM clock frequency. I wonder what does this setting mean?  */
-#define PWM_PERIOD_SLOW 25  /* PWM period (in PWM ticks).    */
-#endif /* PWM_FREQ_SLOW PWM_PERIOD_SLOW */
-
 #if !defined(PWM_FREQ_FAST) || !defined(PWM_PERIOD_FAST)
 /**
  * 8000 RPM is 133Hz
@@ -109,15 +102,16 @@ static int adcDebugReporting = false;
 EXTERN_ENGINE;
 
 static adcsample_t getAvgAdcValue(int index, adcsample_t *samples, int bufDepth, int numChannels) {
-	adcsample_t result = 0;
+	uint32_t result = 0;
 	for (int i = 0; i < bufDepth; i++) {
 		result += samples[index];
 		index += numChannels;
 	}
-	return result / bufDepth;
+
+	// this truncation is guaranteed to not be lossy - the average can't be larger than adcsample_t
+	return static_cast<adcsample_t>(result / bufDepth);
 }
 
-static void adc_callback_slow(ADCDriver *adcp, adcsample_t *buffer, size_t n);
 
 // See https://github.com/rusefi/rusefi/issues/976 for discussion on these values
 #define ADC_SAMPLING_SLOW ADC_SAMPLE_56
@@ -125,7 +119,7 @@ static void adc_callback_slow(ADCDriver *adcp, adcsample_t *buffer, size_t n);
 /*
  * ADC conversion group.
  */
-static ADCConversionGroup adcgrpcfgSlow = { FALSE, 0, adc_callback_slow, NULL,
+static ADCConversionGroup adcgrpcfgSlow = { FALSE, 0, nullptr, NULL,
 /* HW dependent part.*/
 ADC_TwoSamplingDelay_20Cycles,   // cr1
 		ADC_CR2_SWSTART, // cr2
@@ -207,39 +201,7 @@ ADC_TwoSamplingDelay_5Cycles,   // cr1
 
 AdcDevice fastAdc(&adcgrpcfg_fast);
 
-void doSlowAdc(void) {
-
-	efiAssertVoid(CUSTOM_ERR_6658, getCurrentRemainingStack()> 32, "lwStAdcSlow");
-
-#if EFI_INTERNAL_ADC
-
-	/* Starts an asynchronous ADC conversion operation, the conversion
-	 will be executed in parallel to the current PWM cycle and will
-	 terminate before the next PWM cycle.*/
-	slowAdc.conversionCount++;
-	chSysLockFromISR()
-	;
-	if (ADC_SLOW_DEVICE.state != ADC_READY &&
-	ADC_SLOW_DEVICE.state != ADC_COMPLETE &&
-	ADC_SLOW_DEVICE.state != ADC_ERROR) {
-		// todo: why and when does this happen? firmwareError(OBD_PCM_Processor_Fault, "ADC slow not ready?");
-		slowAdc.errorsCount++;
-		chSysUnlockFromISR()
-		;
-		return;
-	}
-
-	adcStartConversionI(&ADC_SLOW_DEVICE, &adcgrpcfgSlow, slowAdc.samples, ADC_BUF_DEPTH_SLOW);
-	chSysUnlockFromISR()
-	;
-#endif /* EFI_INTERNAL_ADC */
-}
-
 #if HAL_USE_PWM
-static void pwmpcb_slow(PWMDriver *pwmp) {
-	(void) pwmp;
-	doSlowAdc();
-}
 
 static void pwmpcb_fast(PWMDriver *pwmp) {
 	efiAssertVoid(CUSTOM_ERR_6659, getCurrentRemainingStack()> 32, "lwStAdcFast");
@@ -311,12 +273,6 @@ int getInternalAdcValue(const char *msg, adc_channel_e hwChannel) {
 }
 
 #if HAL_USE_PWM
-static PWMConfig pwmcfg_slow = { PWM_FREQ_SLOW, PWM_PERIOD_SLOW, pwmpcb_slow, { {
-PWM_OUTPUT_DISABLED, NULL }, { PWM_OUTPUT_DISABLED, NULL }, {
-PWM_OUTPUT_DISABLED, NULL }, { PWM_OUTPUT_DISABLED, NULL } },
-/* HW dependent part.*/
-0, 0 };
-
 static PWMConfig pwmcfg_fast = { PWM_FREQ_FAST, PWM_PERIOD_FAST, pwmpcb_fast, { {
 PWM_OUTPUT_DISABLED, NULL }, { PWM_OUTPUT_DISABLED, NULL }, {
 PWM_OUTPUT_DISABLED, NULL }, { PWM_OUTPUT_DISABLED, NULL } },
@@ -457,33 +413,48 @@ int getSlowAdcCounter() {
 	return slowAdcCounter;
 }
 
-static void adc_callback_slow(ADCDriver *adcp, adcsample_t *buffer, size_t n) {
-	(void) buffer;
-	(void) n;
 
-	ScopePerf perf(PE::AdcCallbackSlow);
+class SlowAdcController : public PeriodicController<256> {
-
+public:
-	/* Note, only in the ADC_COMPLETE state because the ADC driver fires
+	SlowAdcController() 
-	 * an intermediate callback when the buffer is half full. */
+		: PeriodicController("ADC", NORMALPRIO + 5, 200)
-	if (adcp->state == ADC_COMPLETE) {
+	{
-		slowAdc.invalidateSamplesCache();
-
-		efiAssertVoid(CUSTOM_STACK_ADC_6671, getCurrentRemainingStack() > 128, "lowstck#9c");
-
-		/* Calculates the average values from the ADC samples.*/
-		for (int i = 0; i < slowAdc.size(); i++) {
-			int value = getAvgAdcValue(i, slowAdc.samples, ADC_BUF_DEPTH_SLOW, slowAdc.size());
-			adcsample_t prev = slowAdc.values.adc_data[i];
-			float result = (slowAdcCounter == 0) ? value :
-					CONFIG(slowAdcAlpha) * value + (1 - CONFIG(slowAdcAlpha)) * prev;
-
-			slowAdc.values.adc_data[i] = (int)result;
-		}
-		slowAdcCounter++;
-
-		AdcSubscription::UpdateSubscribers();
 	}
-}
+
+	void PeriodicTask(efitime_t nowNt) override {
+		{
+			ScopePerf perf(PE::AdcConversionSlow);
+
+			slowAdc.conversionCount++;
+			msg_t result = adcConvert(&ADC_SLOW_DEVICE, &adcgrpcfgSlow, slowAdc.samples, ADC_BUF_DEPTH_SLOW);
+
+			// If something went wrong - try again later
+			if (result == MSG_RESET || result == MSG_TIMEOUT) {
+				slowAdc.errorsCount++;
+				return;
+			}
+		}
+
+		{
+			ScopePerf perf(PE::AdcProcessSlow);
+
+			slowAdc.invalidateSamplesCache();
+
+			/* Calculates the average values from the ADC samples.*/
+			for (int i = 0; i < slowAdc.size(); i++) {
+				adcsample_t value = getAvgAdcValue(i, slowAdc.samples, ADC_BUF_DEPTH_SLOW, slowAdc.size());
+				adcsample_t prev = slowAdc.values.adc_data[i];
+				float result = (slowAdcCounter == 0) ? value :
+						CONFIG(slowAdcAlpha) * value + (1 - CONFIG(slowAdcAlpha)) * prev;
+
+				slowAdc.values.adc_data[i] = (adcsample_t)result;
+			}
+			slowAdcCounter++;
+
+			AdcSubscription::UpdateSubscribers();
+		}
+	}
+};
 
 static char errorMsgBuff[_MAX_FILLER + 2];
 
@@ -567,6 +538,8 @@ static void configureInputs(void) {
 	setAdcChannelOverrides();
 }
 
+static SlowAdcController slowAdcController;
+
 void initAdcInputs() {
 	printMsg(&logger, "initAdcInputs()");
 	if (ADC_BUF_DEPTH_FAST > MAX_ADC_GRP_BUF_DEPTH)
@@ -606,10 +579,9 @@ void initAdcInputs() {
 #endif /* ADC_CHANNEL_SENSOR */
 
 	slowAdc.init();
-#if HAL_USE_PWM
+
-	pwmStart(EFI_INTERNAL_SLOW_ADC_PWM, &pwmcfg_slow);
+	// Start the slow ADC thread
-	pwmEnablePeriodicNotification(EFI_INTERNAL_SLOW_ADC_PWM);
+	slowAdcController.Start();
-#endif /* HAL_USE_PWM */
 
 	if (CONFIGB(isFastAdcEnabled)) {
 		fastAdc.init();