PR Review Updates by FacioErgoSum · Pull Request #3 · sparkfun/SparkFun_ADE7953_Arduino_Library

FacioErgoSum · 2026-06-02T16:33:03Z

Removed Example 8
Fixed Example 1
changed "rms" to "RMS"
Passed Error codes through all functions
Added bitfield unions and changed const to protected
Added helper functions to header, autoCalibrate and setCurrentClamp
Removed Herobrine

Core driver (src/sfTk/sfDevADE7953.h/.cpp): - Trim implementation-detail block from the header comment. - Add typedef to the PGA gain and ZX edge enums for C compatibility. - Move all register addresses and magic values into the class as protected static members, dropping the sfADE7953 prefix. - Rename the I2C address constant (drop "default"; single fixed address). - Redesign the API to return sfTkError_t with values passed by reference, so communication failures are distinguishable from valid data. - Remove the unused DEBUG_SERIAL_PRINTS block and the redundant per-method null-pointer checks (the Toolkit bus already guards these). - Move isConnected() into the base class; begin() now verifies the device is present before writing any configuration. - Apply sensible defaults in begin() (4x PGA gain on both channels, HPF on). - Rename setZXISource/getZXISource -> setZXISourceChannel/getZXISourceChannel. Bug fixes: - reset() now re-applies the unlock/optimize sequence and defaults (the OPTIMIZE register reverts to POR after a software reset). - setGainV() rejects ADE7953_PGA_GAIN_22 (invalid for the voltage channel). - getCurrentA/B() read the active PGA gain per call, fixing the example bug where a hard-coded multiplier disagreed with the configured gain. New convenience methods: - getCurrentA()/getCurrentB() return RMS current in amps; configurable CT ratio and burden resistor with board defaults. - Float-based digital gain helpers (setDigitalGainIA/IB(float), getDigitalGainIA/IB(float&)). Wrapper + metadata: - SparkFun_ADE7953.h: drop @code usage blocks, ping the device before begin(), pass the I2C address by value, isConnected() now inherited from the base class. - library.properties: remove the unneeded includes line. Examples: - Example 1 simplified (retry instead of freeze, no memset/rolling average, uses getCurrentA()). - All examples updated to the error-code API and new method names. - Example 7 reworked to demonstrate error handling and a 64-bit calibration accumulator (fixes the overflow), with a note on the AIRMSOS squared domain. - Example 8 removed (it duplicated Examples 2 and 6).

- Rename all *Rms* identifiers to *RMS* (acronym): getIRMSA/B, setIRMSOffsetA/B, getIRMSOffsetA/B, and the kReg*RMS* register constants. - Add sfe_ade7953_clamp_t plus setCurrentClamp(): pass ADE7953_CLAMP_ECS1030 or ADE7953_CLAMP_SCT013 to apply the turns ratio and a suitable PGA gain, or setCurrentClamp(float) to enter a custom turns ratio. - Add autoCalibrateA()/autoCalibrateB(numSamples) that average no-load samples into a software baseline; getCurrentA()/getCurrentB() remove it in the squared domain (sqrt(reading^2 - baseline^2)) — physically correct and needs no datasheet scaling. Add clearCalibration(). - Example 2 demonstrates setCurrentClamp(); Example 7 reworked to use autoCalibrateA().

- README.md: long-form SparkFun-style readme with banner reference, badges, functionality overview, wiring, and a full usage guide (reading current, error handling, clamp selection, calibration, gain, peaks, interrupts, zero-crossing) plus example links. - keywords.txt: Arduino IDE syntax highlighting for all public methods, types, and constants. - library.json: PlatformIO manifest (with SparkFun Toolkit dependency). - docs/doxygen/doxygen-config: Doxygen config (README as main page, src input). - .github/workflows/build-deploy-ghpages.yml: build docs and deploy to Pages. - .github/workflows/test-compile-sketch.yml: cross-compile Example 01 across AVR/ESP32/ESP8266/SAMD/mbed/RP2040/RP2350/STM32, installing the Toolkit dep.

sfe-SparkFro · 2026-06-11T15:34:22Z

+    for (int i = 0; i < AVERAGE_WINDOW; i++)
+        irmsBuffer[i] = 0;
+}
+
+// Convert a PGA gain enum to its numeric multiplier for display.
+int pgaMultiplier(sfe_ade7953_pga_gain_t gain)
+{
+    switch (gain)
+    {
+    case ADE7953_PGA_GAIN_1:
+        return 1;
+    case ADE7953_PGA_GAIN_2:
+        return 2;
+    case ADE7953_PGA_GAIN_4:
+        return 4;
+    case ADE7953_PGA_GAIN_8:
+        return 8;
+    case ADE7953_PGA_GAIN_16:
+        return 16;
+    case ADE7953_PGA_GAIN_22:
+        return 22;
+    default:
+        return -1;
+    }


This should be a method in the class

sfe-SparkFro · 2026-06-11T15:43:33Z

+    /// @brief Convert a PGA gain enum value to its numeric multiplier (1, 2, 4, 8, 16, 22).
+    static float pgaGainToMultiplier(sfe_ade7953_pga_gain_t gain);


Ah, it already exists in the class! Make this public instead of protected

sfe-SparkFro · 2026-06-11T15:48:19Z

+    /// @brief Convert a PGA gain enum value to its numeric multiplier (1, 2, 4, 8, 16, 22).
+    static float pgaGainToMultiplier(sfe_ade7953_pga_gain_t gain);


This should return an integer, not a float

sfe-SparkFro · 2026-06-11T15:50:30Z

 // Helper to print the PGA gain as a human-readable string.
 void printGain(sfe_ade7953_pga_gain_t gain)
 {
    switch (gain)
    {
    case ADE7953_PGA_GAIN_1:
        Serial.print("1x");
        break;
    case ADE7953_PGA_GAIN_2:
        Serial.print("2x");
        break;
    case ADE7953_PGA_GAIN_4:
        Serial.print("4x");
        break;
    case ADE7953_PGA_GAIN_8:
        Serial.print("8x");
        break;
    case ADE7953_PGA_GAIN_16:
        Serial.print("16x");
        break;
    case ADE7953_PGA_GAIN_22:
        Serial.print("22x");
        break;
    default:
        Serial.print("Unknown");
        break;
    }
 }


Instead of this helper method in the example, use the pgaGainToMultiplier() method from the class

sfe-SparkFro · 2026-06-11T15:55:17Z

+    // Set an overcurrent threshold (raw ADC counts). Adjust for your expected current range.
    uint32_t overcurrentThreshold = 0x100000;
    mySensor.setOvercurrentLevel(overcurrentThreshold);
    Serial.print("Overcurrent threshold set to: 0x");
    Serial.println(overcurrentThreshold, HEX);


Make a helper method that allows the user to set the threshold in Amps, not a raw register value.

sfe-SparkFro · 2026-06-11T16:30:39Z

 void doZeroCalibration()
 {
    const int NUM_SAMPLES = 50;

-    Serial.println(F("\nZero calibration — keep CT clamped with NO current flowing."));
-    Serial.print(F("Collecting "));
-    Serial.print(NUM_SAMPLES);
-    Serial.println(F(" samples..."));
+    Serial.println(F("\nZero calibration - keep CT clamped with NO current flowing."));

    uint64_t sum = 0;
    for (int i = 0; i < NUM_SAMPLES; i++)
    {
-        sum += mySensor.getIRmsA();
+        uint32_t sample = 0;
+        mySensor.getIRMSA(sample);
+        sum += sample;
        delay(50);
-        if ((i + 1) % 10 == 0)
-        {
-            Serial.print(F("  "));
-            Serial.print(i + 1);
-            Serial.print(F("/"));
-            Serial.println(NUM_SAMPLES);
-        }
    }

-    zeroOffset = (int32_t)(sum / NUM_SAMPLES);
+    zeroOffset = (uint32_t)(sum / NUM_SAMPLES);
    resetBuffer();

    Serial.print(F("Zero offset set to "));
    Serial.print(zeroOffset);
    Serial.println(F(". Readings will subtract this baseline.\n"));
 }


Is this function needed? Can't you just call autoCalibrateA() instead?

sfe-SparkFro · 2026-06-11T16:34:27Z

+     * @return false If any initialization step fails.
     */
-    bool begin(const uint8_t &address = kADE7953DefaultAddr, TwoWire &wirePort = Wire)
+    bool begin(uint8_t address = kI2CAddress, TwoWire &wirePort = Wire)


There's only 1 address option for this chip, so remove the address parameter

sfe-SparkFro · 2026-06-11T16:36:16Z

        if (_theI2CBus.ping() != ksfTkErrOk)
            return false;


This check isn't needed, sfDevADE7953::begin() now handles all the necessary checks

sfe-SparkFro · 2026-06-11T16:40:04Z

+    sfTkError_t setCurrentClamp(sfe_ade7953_clamp_t clamp);
+
+    /// @brief Configure a custom current clamp by its turns ratio.
+    /// @details Use this for a clamp that is not one of the presets. Only the CT ratio is changed;
+    /// the PGA gain is left as-is (set it with setGainIA()/setGainIB() if needed).
+    /// @param turnsRatio Turns ratio of the current transformer (e.g. 2000.0 for 30A:15mA).
+    void setCurrentClamp(float turnsRatio);


These should not have the same name. The first one is fine, second one should be something like setTurnsRatio

sfe-SparkFro · 2026-06-11T16:43:34Z

+    // --- Current-to-amps conversion parameters (sensible defaults for the Qwiic board) ---
+    float _ctRatio = 2000.0f;          ///< CT turns ratio (30A:15mA = 2000:1).
+    float _burdenResistor = 5.6f;      ///< Burden resistor in ohms.
+    float _fullScaleCode = 5928256.0f; ///< ADE7953 full-scale IRMS code.
+    float _fullScaleVRMS = 0.35355f;   ///< Full-scale input RMS voltage (0.5 V peak / sqrt(2)).


Shouldn't there be separate values for each channel? A user could connect 2 different current clamps to each channel

Per-channel current clamp support: - Split the shared CT turns ratio into independent Channel A / Channel B ratios so each channel can use a different clamp (or measure directly). - Add setCurrentClampA/B, setCurrentTransformerRatioA/B, and getCurrentTransformerRatioA/B; existing single-arg methods now apply to both channels for backward compatibility. - Update keywords.txt, README, and Example02 to demonstrate per-channel use. Align with SparkFun_Arduino_Library_HowTo: - Wire up the doxygen-awesome theme: add doxygen-awesome-css submodule (v2.4.2), custom header.html/custom.css, and the matching doxygen-config HTML keys + PROJECT_LOGO. - Fix license-header wording in sfDevADE7953.cpp and SparkFun_ADE7953.h ("All rights reserved." -> MIT release wording). - Use the prescribed README License Information block. - Rename Example09 -> Example08 to close the numbering gap. - Fix "SparkFun Electronic" typo in Example01. - Add add_issue_to_project.yml issue-triage workflow. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Geoff McIntyre added 4 commits June 1, 2026 20:16

Update library.properties

3ef2d46

Fixed Case and gain values

9ed1eed

FacioErgoSum requested a review from sfe-SparkFro June 2, 2026 16:33

Geoff McIntyre added 2 commits June 2, 2026 18:19

Removed context files. Added License

341957b

sfe-SparkFro requested changes Jun 11, 2026

View reviewed changes

Geoff McIntyre and others added 2 commits June 15, 2026 10:47

Sets Gain to default 16x

a707d76

FacioErgoSum merged commit 2c24c46 into main Sep 1, 2026
9 checks passed

FacioErgoSum deleted the development branch September 1, 2026 19:29

Navigation Menu

Uh oh!

PR Review Updates - #3

PR Review Updates#3
FacioErgoSum merged 8 commits into
mainfrom
development

FacioErgoSum commented Jun 2, 2026 •

edited

Loading

Uh oh!

sfe-SparkFro Jun 11, 2026

Uh oh!

sfe-SparkFro Jun 11, 2026

Uh oh!

sfe-SparkFro Jun 11, 2026

Uh oh!

sfe-SparkFro Jun 11, 2026

Uh oh!

sfe-SparkFro Jun 11, 2026

Uh oh!

sfe-SparkFro Jun 11, 2026

Uh oh!

sfe-SparkFro Jun 11, 2026

Uh oh!

sfe-SparkFro Jun 11, 2026

Uh oh!

sfe-SparkFro Jun 11, 2026

Uh oh!

sfe-SparkFro Jun 11, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		/// @brief Convert a PGA gain enum value to its numeric multiplier (1, 2, 4, 8, 16, 22).
		static float pgaGainToMultiplier(sfe_ade7953_pga_gain_t gain);

Uh oh!

Conversation

FacioErgoSum commented Jun 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

FacioErgoSum commented Jun 2, 2026 •

edited

Loading