[atm90e32] Verify offset calibration writes (#18701)

Co-authored-by: pre-commit-ci-lite[bot] <117423508+pre-commit-ci-lite[bot]@users.noreply.github.com>

esphome/components/atm90e32/atm90e32.h

@@ -13,6 +13,40 @@
 
 namespace esphome::atm90e32 {
 
+inline bool offset_register_value_matches(uint16_t actual, int16_t expected) {
+  return actual == static_cast<uint16_t>(expected);
+}
+
+struct OffsetCalibration {
+  int16_t first_offset{0};
+  int16_t second_offset{0};
+};
+
+static_assert(sizeof(OffsetCalibration[3]) == 12, "Offset calibration preference layout must remain compatible");
+
+enum class OffsetCalibrationType : uint8_t {
+  OFFSET_CALIBRATION_TYPE_VOLTAGE_CURRENT,
+  OFFSET_CALIBRATION_TYPE_POWER,
+};
+
+struct OffsetRestoreState {
+  bool restored;
+  bool values_verified;
+};
+
+inline OffsetRestoreState resolve_offset_restore_state(bool has_stored_values, bool initial_values_verified,
+                                                       bool fallback_values_verified) {
+  if (initial_values_verified)
+    return {has_stored_values, true};
+  return {false, fallback_values_verified};
+}
+
+inline void prepare_offset_rollback(const OffsetCalibration (&previous)[3], bool had_stored_values,
+                                    OffsetCalibration (&rollback)[3]) {
+  for (uint8_t phase = 0; phase < 3; phase++)
+    rollback[phase] = had_stored_values ? previous[phase] : OffsetCalibration{};
+}
+
 class ATM90E32Component final : public PollingComponent,
                                 public spi::SPIDevice<spi::BIT_ORDER_MSB_FIRST, spi::CLOCK_POLARITY_HIGH,
                                                       spi::CLOCK_PHASE_TRAILING, spi::DATA_RATE_1MHZ> {
@@ -71,19 +105,19 @@ class ATM90E32Component final : public PollingComponent,
     this->has_config_current_gain_[phase] = true;
   }
   void set_voltage_offset(uint8_t phase, int16_t offset) {
-    this->offset_phase_[phase].voltage_offset_ = offset;
+    this->offset_phase_[phase].first_offset = offset;
     this->has_config_voltage_offset_[phase] = true;
   }
   void set_current_offset(uint8_t phase, int16_t offset) {
-    this->offset_phase_[phase].current_offset_ = offset;
+    this->offset_phase_[phase].second_offset = offset;
     this->has_config_current_offset_[phase] = true;
   }
   void set_active_power_offset(uint8_t phase, int16_t offset) {
-    this->power_offset_phase_[phase].active_power_offset = offset;
+    this->power_offset_phase_[phase].first_offset = offset;
     this->has_config_active_power_offset_[phase] = true;
   }
   void set_reactive_power_offset(uint8_t phase, int16_t offset) {
-    this->power_offset_phase_[phase].reactive_power_offset = offset;
+    this->power_offset_phase_[phase].second_offset = offset;
     this->has_config_reactive_power_offset_[phase] = true;
   }
   void set_freq_sensor(sensor::Sensor *freq_sensor) { freq_sensor_ = freq_sensor; }
@@ -171,16 +205,16 @@ class ATM90E32Component final : public PollingComponent,
   float get_chip_temperature_();
   bool get_publish_interval_flag_() { return publish_interval_flag_; };
   void set_publish_interval_flag_(bool flag) { publish_interval_flag_ = flag; };
-  void restore_offset_calibrations_();
-  void restore_power_offset_calibrations_();
+  void restore_offset_calibrations_(OffsetCalibrationType type);
   void restore_gain_calibrations_();
-  void save_offset_calibration_to_memory_();
   void save_gain_calibration_to_memory_();
-  void save_power_offset_calibration_to_memory_();
-  void write_offsets_to_registers_(uint8_t phase, int16_t voltage_offset, int16_t current_offset);
-  void write_power_offsets_to_registers_(uint8_t phase, int16_t p_offset, int16_t q_offset);
+  void finish_offset_calibration_(const OffsetCalibration (&previous)[3], bool previous_restored,
+                                  bool previous_using_saved, OffsetCalibrationType type);
+  void write_offsets_to_registers_(uint8_t phase, int16_t first_offset, int16_t second_offset,
+                                   OffsetCalibrationType type);
   void write_gains_to_registers_();
   bool verify_gain_writes_();
+  bool verify_offset_writes_(OffsetCalibrationType type);
   bool validate_spi_read_(uint16_t expected, const char *context = nullptr);
   void log_calibration_status_();
   const char *get_calibration_id_();
@@ -219,19 +253,10 @@ class ATM90E32Component final : public PollingComponent,
     uint32_t cumulative_reverse_active_energy_{0};
   } phase_[3];
 
-  struct OffsetCalibration {
-    int16_t voltage_offset_{0};
-    int16_t current_offset_{0};
-  } offset_phase_[3];
-
+  OffsetCalibration offset_phase_[3];
   OffsetCalibration config_offset_phase_[3];
-
-  struct PowerOffsetCalibration {
-    int16_t active_power_offset{0};
-    int16_t reactive_power_offset{0};
-  } power_offset_phase_[3];
-
-  PowerOffsetCalibration config_power_offset_phase_[3];
+  OffsetCalibration power_offset_phase_[3];
+  OffsetCalibration config_power_offset_phase_[3];
 
   struct GainCalibration {
     uint16_t voltage_gain{1};
@@ -265,6 +290,8 @@ class ATM90E32Component final : public PollingComponent,
   bool enable_offset_calibration_{false};
   bool enable_gain_calibration_{false};
   const char *instance_id_{nullptr};
+  bool has_stored_offset_calibration_{false};
+  bool has_stored_power_offset_calibration_{false};
   bool restored_offset_calibration_{false};
   bool restored_power_offset_calibration_{false};
   bool restored_gain_calibration_{false};

tests/components/atm90e32/__init__.py

@@ -0,0 +1,5 @@
+from tests.testing_helpers import ComponentManifestOverride
+
+
+def override_manifest(manifest: ComponentManifestOverride) -> None:
+    manifest.dependencies = manifest.dependencies + ["sensor", "spi"]

tests/components/atm90e32/offset_register_verification_test.cpp

@@ -0,0 +1,62 @@
+#include <gtest/gtest.h>
+
+#include "esphome/components/atm90e32/atm90e32.h"
+
+namespace esphome::atm90e32::testing {
+
+TEST(ATM90E32OffsetRegisterVerification, AcceptsExactSignedReadback) {
+  EXPECT_TRUE(offset_register_value_matches(0x007B, 123));
+  EXPECT_TRUE(offset_register_value_matches(0xFF85, -123));
+}
+
+TEST(ATM90E32OffsetRegisterVerification, RejectsMismatchedReadback) {
+  EXPECT_FALSE(offset_register_value_matches(0x007C, 123));
+  EXPECT_FALSE(offset_register_value_matches(0xFF84, -123));
+}
+
+TEST(ATM90E32OffsetRestoreState, ReportsVerifiedStoredValuesAsRestored) {
+  const auto state = resolve_offset_restore_state(true, true, false);
+
+  EXPECT_TRUE(state.restored);
+  EXPECT_TRUE(state.values_verified);
+}
+
+TEST(ATM90E32OffsetRestoreState, ReportsVerifiedConfigFallbackAsNotRestored) {
+  const auto state = resolve_offset_restore_state(true, false, true);
+
+  EXPECT_FALSE(state.restored);
+  EXPECT_TRUE(state.values_verified);
+}
+
+TEST(ATM90E32OffsetRestoreState, ReportsFailedConfigFallbackAsUnverified) {
+  const auto state = resolve_offset_restore_state(true, false, false);
+
+  EXPECT_FALSE(state.restored);
+  EXPECT_FALSE(state.values_verified);
+}
+
+TEST(ATM90E32OffsetRestoreState, ReportsConfigWithoutStoredValuesAsNotRestored) {
+  const auto state = resolve_offset_restore_state(false, true, false);
+
+  EXPECT_FALSE(state.restored);
+  EXPECT_TRUE(state.values_verified);
+}
+
+TEST(ATM90E32OffsetPersistence, RollsBackStoredValuesOrZeroSentinel) {
+  const OffsetCalibration previous[3]{{1, -1}, {2, -2}, {3, -3}};
+  OffsetCalibration rollback[3]{};
+
+  prepare_offset_rollback(previous, true, rollback);
+  for (uint8_t phase = 0; phase < 3; phase++) {
+    EXPECT_EQ(rollback[phase].first_offset, previous[phase].first_offset);
+    EXPECT_EQ(rollback[phase].second_offset, previous[phase].second_offset);
+  }
+
+  prepare_offset_rollback(previous, false, rollback);
+  for (const auto &phase : rollback) {
+    EXPECT_EQ(phase.first_offset, 0);
+    EXPECT_EQ(phase.second_offset, 0);
+  }
+}
+
+}  // namespace esphome::atm90e32::testing