From d1fb610358dcc114577712197f802971a03fa832 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Metrich?= Date: Thu, 1 Oct 2026 07:50:40 +0200 Subject: [PATCH 1/4] perf: smaller relay code, relay pin writes no longer clobber the load pins With two relays, basic is 396 bytes smaller (11566 -> 11170): - proceed_relays() reads the TEMA average once, not before every relay; proceed_relay() had two identical branches for the import threshold - try_turnON() and try_turnOFF() are one try_switch(bool) - setPinState() works out the port and the mask once - EWMA_average::addValue() and getAverageT() are no longer inlined setPinState() used a read-modify-write of PORTx from loop(). Had the ADC ISR changed a load pin of the same port in between, the old load state would have been written back, until the next decision. It now toggles the relay's bit through PINx: a single write, which leaves the other bits alone, with no need to disable interrupts. Only possible with a relay on the loads' port (D2-D7), so not with the default pins. Builds without relays are unchanged. Co-Authored-By: Claude Opus 5.5 --- Mk2_3phase_RFdatalog_temp/ewma_avg.hpp | 13 +++-- Mk2_3phase_RFdatalog_temp/utils_pins.h | 22 +++++--- Mk2_3phase_RFdatalog_temp/utils_relay.h | 70 +++++++------------------ 3 files changed, 43 insertions(+), 62 deletions(-) diff --git a/Mk2_3phase_RFdatalog_temp/ewma_avg.hpp b/Mk2_3phase_RFdatalog_temp/ewma_avg.hpp index bb332c0c..94cec660 100644 --- a/Mk2_3phase_RFdatalog_temp/ewma_avg.hpp +++ b/Mk2_3phase_RFdatalog_temp/ewma_avg.hpp @@ -3,7 +3,7 @@ * @author Frédéric Metrich (frederic.metrich@live.fr) * @brief This file implements an Exponentially Weighted Moving Average template class * @version 0.1 - * @date 2024-02-27 + * @date 2026-10-01 * * @section description Description * The Exponentially Weighted Moving Average (EWMA) is a quantitative or statistical measure used to model or describe a time series. @@ -27,7 +27,7 @@ * This comes with some restrictions on the alpha parameter, but the benefit of full integer math wins * on the side-drawback. * - * @copyright Copyright (c) 2024 + * @copyright Copyright (c) 2024-2026 * */ @@ -114,8 +114,11 @@ class EWMA_average * Moving Average (EMA), Double EMA (DEMA), and Triple EMA (TEMA). * * @param input The new input value to process. + * + * @note Not inlined: in a large caller such as loop(), the inlined int32 arithmetic costs + * more flash than the call. */ - void addValue(int32_t input) + __attribute__((noinline)) void addValue(int32_t input) { ema_raw = ema_raw - ema + input; ema = ema_raw >> round_up_to_power_of_2(A); @@ -151,8 +154,10 @@ class EWMA_average * @brief Get the Triple Exponentially Weighted Moving Average (TEMA). * * @return auto The TEMA value. + * + * @note Not inlined: it is read in several places (relay decisions, datalog output). */ - auto getAverageT() const + __attribute__((noinline)) auto getAverageT() const { return 3 * (ema - ema_ema) + ema_ema_ema; } diff --git a/Mk2_3phase_RFdatalog_temp/utils_pins.h b/Mk2_3phase_RFdatalog_temp/utils_pins.h index 4efa4400..6c5d10dc 100644 --- a/Mk2_3phase_RFdatalog_temp/utils_pins.h +++ b/Mk2_3phase_RFdatalog_temp/utils_pins.h @@ -3,7 +3,7 @@ * @author Frédéric Metrich (frederic.metrich@live.fr) * @brief Some utility functions for pins manipulation * @version 0.1 - * @date 2026-01-29 + * @date 2026-10-01 * * @copyright Copyright (c) 2023-2026 * @@ -71,16 +71,22 @@ void constexpr togglePin(const uint8_t pin) * * @param pin pin to change [2..13] * @param bState state to be set + * + * @details For a pin only known at run time (a relay). The ADC ISR writes the load pins, + * possibly on the same port, so the port must not be read-modified-written here: + * the pin is toggled through PINx instead, where writing a 1 toggles that bit of + * PORTx and writing a 0 leaves the others alone. Only the caller changes this pin, + * so reading its state first is safe. */ -inline constexpr void setPinState(const uint8_t pin, const bool bState) +inline void setPinState(const uint8_t pin, const bool bState) { - if (bState) - { - setPinON(pin); - } - else + const volatile uint8_t &port{ (pin < 8) ? PORTD : ((pin < 14) ? PORTB : PORTC) }; + volatile uint8_t &toggle{ (pin < 8) ? PIND : ((pin < 14) ? PINB : PINC) }; + const uint8_t mask{ static_cast< uint8_t >(1U << ((pin < 8) ? pin : ((pin < 14) ? pin - 8 : pin - 14))) }; + + if (static_cast< bool >(port & mask) != bState) { - setPinOFF(pin); + toggle = mask; } } diff --git a/Mk2_3phase_RFdatalog_temp/utils_relay.h b/Mk2_3phase_RFdatalog_temp/utils_relay.h index cbff261f..d281655f 100644 --- a/Mk2_3phase_RFdatalog_temp/utils_relay.h +++ b/Mk2_3phase_RFdatalog_temp/utils_relay.h @@ -3,7 +3,7 @@ * @author Frédéric Metrich (frederic.metrich@live.fr) * @brief Some utility functions for the relay output feature * @version 0.1 - * @date 2026-01-29 + * @date 2026-10-01 * * @copyright Copyright (c) 2023-2026 * @@ -173,25 +173,14 @@ class relayOutput if (currentAvgPower < surplusThreshold || isOverrideActive) { bit_clear(overrideBitmask, relay_pin); // Clear override bit if it was set - return try_turnON(); + return try_switch(true); } - // Handle both positive and negative import thresholds - if (importThreshold >= 0) + // Positive threshold: turn OFF when importing > threshold (normal mode). + // Negative threshold: turn OFF when surplus < abs(threshold) (battery mode). + if (currentAvgPower > importThreshold) { - // Positive threshold: turn OFF when importing > threshold (normal mode) - if (currentAvgPower > importThreshold) - { - return try_turnOFF(); - } - } - else - { - // Negative threshold: turn OFF when surplus < abs(threshold) (battery mode) - if (currentAvgPower > importThreshold) // importThreshold is negative, so this checks surplus < abs(threshold) - { - return try_turnOFF(); - } + return try_switch(false); } return false; @@ -234,55 +223,34 @@ class relayOutput /** * @brief Force the relay OFF when diversion is disabled - * + * * @return bool True if state has changed * @details This method attempts to turn OFF the relay, respecting the minimum ON time constraint */ bool forceOFF() const { - return try_turnOFF(); + return try_switch(false); } private: /** - * @brief Turn ON the relay if the 'time' condition is met + * @brief Turn the relay ON or OFF if the 'time' condition is met * + * @param on True to turn the relay ON, false to turn it OFF * @return bool True if state has changed */ - bool try_turnON() const + bool try_switch(const bool on) const { - if (relayIsON || duration < minOFF) + if ((relayIsON == on) || (duration < (on ? minOFF : minON))) { return false; } - setPinON(relay_pin); + setPinState(relay_pin, on); - DBUGLN(F("Relay turned ON!")); + DBUGLN(on ? F("Relay turned ON!") : F("Relay turned OFF!")); - relayIsON = true; - duration = 0; - - return true; - } - - /** - * @brief Turn OFF the relay if the 'time' condition is met - * - * @return bool True if state has changed - */ - bool try_turnOFF() const - { - if (!relayIsON || duration < minON) - { - return false; - } - - setPinOFF(relay_pin); - - DBUGLN(F("Relay turned OFF!")); - - relayIsON = false; + relayIsON = on; duration = 0; return true; @@ -434,13 +402,15 @@ class RelayEngine return; } - if (ewma_average.getAverageT() > 0) + const auto currentAvgPower{ ewma_average.getAverageT() }; + + if (currentAvgPower > 0) { // Currently importing, try to turn OFF some relays uint8_t idx{ N }; do { - if (relay[--idx].proceed_relay(ewma_average.getAverageT(), overrideBitmask)) + if (relay[--idx].proceed_relay(currentAvgPower, overrideBitmask)) { settle_change = 60; return; @@ -453,7 +423,7 @@ class RelayEngine uint8_t idx{ 0 }; do { - if (relay[idx].proceed_relay(ewma_average.getAverageT(), overrideBitmask)) + if (relay[idx].proceed_relay(currentAvgPower, overrideBitmask)) { settle_change = 60; return; From 0e5eaefbe7adede8cf55a5977fd39f3d0aa9732a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Metrich?= Date: Thu, 1 Oct 2026 08:26:14 +0200 Subject: [PATCH 2/4] docs: say which pin helpers are safe from loop() setPinsON() and setPinsOFF() read-modify-write the ports: only safe from the ADC ISR, which cannot be interrupted. setPinON() and setPinOFF() are atomic only for a pin known at compile time. From loop(), setPinState() and togglePin() write through PINx. Co-Authored-By: Claude Opus 5.5 --- Mk2_3phase_RFdatalog_temp/utils_pins.h | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/Mk2_3phase_RFdatalog_temp/utils_pins.h b/Mk2_3phase_RFdatalog_temp/utils_pins.h index 6c5d10dc..51d8cf46 100644 --- a/Mk2_3phase_RFdatalog_temp/utils_pins.h +++ b/Mk2_3phase_RFdatalog_temp/utils_pins.h @@ -94,6 +94,9 @@ inline void setPinState(const uint8_t pin, const bool bState) * @brief Set the Pin state to ON for the specified pin * * @param pin pin to change [2..13] + * + * @warning Atomic (a single sbi/cbi) only for a pin known at compile time. For a pin known at + * run time it is a read-modify-write of the port: from loop(), use setPinState(). */ inline constexpr void setPinON(const uint8_t pin) { @@ -115,6 +118,10 @@ inline constexpr void setPinON(const uint8_t pin) * @brief Set the Pins state to ON * * @param pins The pins to change + * + * @warning Read-modify-write of PORTD and PORTB: only safe from the ADC ISR (its sole caller), + * which cannot be interrupted. From loop(), use setPinState() or togglePin(): an ISR + * landing in the middle would have its load pin changes written back to the old state. */ inline void setPinsON(const uint16_t pins) { @@ -126,6 +133,9 @@ inline void setPinsON(const uint16_t pins) * @brief Set the Pin state to OFF for the specified pin * * @param pin pin to change [2..13] + * + * @warning Atomic (a single sbi/cbi) only for a pin known at compile time. For a pin known at + * run time it is a read-modify-write of the port: from loop(), use setPinState(). */ inline constexpr void setPinOFF(const uint8_t pin) { @@ -147,6 +157,10 @@ inline constexpr void setPinOFF(const uint8_t pin) * @brief Set the Pins state to OFF * * @param pins The pins to change + * + * @warning Read-modify-write of PORTD and PORTB: only safe from the ADC ISR (its sole caller), + * which cannot be interrupted. From loop(), use setPinState() or togglePin(): an ISR + * landing in the middle would have its load pin changes written back to the old state. */ inline void setPinsOFF(const uint16_t pins) { From 5fb23f1ffadcce51f7745db0e0f2d48b8ed19301 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Metrich?= Date: Thu, 1 Oct 2026 08:33:56 +0200 Subject: [PATCH 3/4] docs: the real reason EWMA_average::addValue() is not inlined Inlined, a static instance's state is reached with absolute lds/sts, 4 bytes per byte accessed; out of line, through the this pointer with ld/st/ldd/std, 2 bytes. Not register spills, as the comment said. Co-Authored-By: Claude Opus 5.5 --- Mk2_3phase_RFdatalog_temp/ewma_avg.hpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Mk2_3phase_RFdatalog_temp/ewma_avg.hpp b/Mk2_3phase_RFdatalog_temp/ewma_avg.hpp index 94cec660..39204736 100644 --- a/Mk2_3phase_RFdatalog_temp/ewma_avg.hpp +++ b/Mk2_3phase_RFdatalog_temp/ewma_avg.hpp @@ -115,8 +115,9 @@ class EWMA_average * * @param input The new input value to process. * - * @note Not inlined: in a large caller such as loop(), the inlined int32 arithmetic costs - * more flash than the call. + * @note Not inlined, even with a single caller: on AVR, inlined code reaches a static + * instance's six int32 with absolute lds/sts (4 bytes per byte accessed), the + * function through its this pointer with ld/st/ldd/std (2 bytes) - 78 bytes less. */ __attribute__((noinline)) void addValue(int32_t input) { From 3aeecaed3b0bc00e63270b0d9d710b9ef2372ade Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Fr=C3=A9d=C3=A9ric=20Metrich?= Date: Thu, 1 Oct 2026 08:40:18 +0200 Subject: [PATCH 4/4] docs: why EWMA_average::getAverageT() is not inlined Two callers in a basic build, and each inlined copy would read the static filter's averages with absolute lds (4 bytes per byte), against ld/ldd through this (2 bytes): 70 bytes less out of line. Co-Authored-By: Claude Opus 5.5 --- Mk2_3phase_RFdatalog_temp/ewma_avg.hpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Mk2_3phase_RFdatalog_temp/ewma_avg.hpp b/Mk2_3phase_RFdatalog_temp/ewma_avg.hpp index 39204736..4525ab3c 100644 --- a/Mk2_3phase_RFdatalog_temp/ewma_avg.hpp +++ b/Mk2_3phase_RFdatalog_temp/ewma_avg.hpp @@ -156,7 +156,9 @@ class EWMA_average * * @return auto The TEMA value. * - * @note Not inlined: it is read in several places (relay decisions, datalog output). + * @note Not inlined: it is read in several places (relay decisions, datalog output), and + * each inlined copy would read a static instance's averages with absolute lds + * (4 bytes per byte), against ld/ldd through this (2 bytes) - 70 bytes less. */ __attribute__((noinline)) auto getAverageT() const {