Repository navigation
feat: left-aligned ADC and circular channel list (#121) - #127
Merged
Merged
Conversation
Replace switch-case logic in ADC interrupt with circular linked list for improved performance and reduced code size. Based on florentbr's optimization suggestion #1 from issue #113. - Add adc_ctx_t structure with minimal memory footprint - Implement circular linked list for 6 ADC channels - Eliminate branching in time-critical ISR code - Maintain exact same functionality as original implementation
Reorganize code structure for better maintainability: - Remove duplicate ADC definitions from top of file - Keep all ADC optimization code together near the ISR - Improve code organization and readability No functional changes, just code organization improvement.
- Changed DC offset from per-cycle LPF to per-sample EMA filter - Use Q16.16 fixed-point format for efficient calculation - Enable ADLAR=1 for left-aligned ADC results - Simplify voltage/current sample processing - Reduce computational overhead in ISR
Move opening brace of the else branch to a new line for consistent brace style in processing.cpp
…n metadata - Replace two type-specific overloads with a single constexpr template<typename T, size_t N> initializeArray(T (&array)[N], T value) to remove duplication and support any array element type. - Expand and clarify Doxygen comments for initializeArray (usage, template params, notes). - Update version.h CURRENT_TIME and reset
Closed
- Add mult_asm.h with optimized 16x16->32 signed multiplication (~3x faster) - Integrate assembly multiplication in processCurrentRawSample() and processVoltage() - Fix ADMUX timing issue (#125): move ADMUX assignment to end of ISR - Change CT filter condition to 'if constexpr' for guaranteed dead code elimination - Add unit tests for assembly multiplication functions Performance improvements: - ISR multiplication: ~15-20 cycles vs ~50+ cycles (library calls) - Zero overhead for disabled CT filtering (compile-time elimination) - Proper ADMUX timing: 128+ CPU cycles after trigger event per datasheet Based on florentbr's optimization suggestions and ATmega328p datasheet requirements.
- Add multU16x16_to32 unsigned multiplication function - Change l_sum_Vsquared from int32_t to uint32_t (V² always positive) - Rename mult16x16_to32 to multS16x16_to32 for clarity - Add comprehensive test coverage for unsigned multiplication - Add voltage accumulation overflow tests (220 lines) - Validates current shift strategy is safe for all datalog periods - Proves uint32_t provides 2x headroom vs int32_t (4.3B vs 2.1B) Related: #121, #125, PR #127
Make spacing consistent around template parameters and static_cast uses, and tidy up stray blank/whitespace in test_main.cpp for the voltage accumulation overflow tests.
FredM67
marked this pull request as ready for review
October 17, 2025 16:50
…rocessCurrentRawSample Add inline declarations (uint16_t sample parameter) in both header sections so the function signatures match the always_inline/optimized declarations and the Doxygen variants, avoiding mismatches.
…hase into 121-suggestions-1-2
…t scaling and tests - In ISR-heavy paths, reduce voltage/current samples by >>2 (to 14-bit / x16 scale) prior to multS16x16_to32 to improve efficiency on 8-bit MCUs. - Update scaling shifts to match the new pre-shift: - real power multiplication: use filt >>2 then mult, final shift >>8 - V² accumulation: use filt >>2 then mult, final shift >>8 for <=10s or >>12 for >10s (effectively reducing previous right-shifts by 4 bits while preserving numeric scale) - Update test/embedded/test_voltage_accumulation to use the 14-bit pre-shift strategy, adjust test cases and messages (including overflow demo) and refresh conclusions.
Owner
Author
|
@florentbr Good news, at least for voltage on L1. The value are still correct ;-) From time to time, there's a value of "0.81" for V2 and V3, even nothing is wired. Same exact effet with the "original" version :D |
* fix: update ADMUX at end of ADC ISR to meet ATmega328P timing Move ADMUX write to the end of the ADC_vect ISR so at least 128 CPU cycles have passed since the trigger event (per ATmega328P datasheet). Prevents premature channel switching and ensures correct sampling timing. * #126 (comment)
Add comprehensive test suite for the DC offset filter changes in PR #127: - 20 unit tests covering initialization, tracking, edge cases, precision - 5 comparison tests proving equivalence with old filter implementation - Documents int16_t overflow at ADC max (1023) - safe range is 0-1022 - Verifies new filter tracks same as old (within 5 ADC counts) - Documents slower time constant (~400 vs ~200 cycles) for better noise immunity Key findings: - Mathematical equivalence confirmed between old and new implementations - New filter has no explicit limits (vs old 412-612 range) - Normal 230V operation uses ADC ~96-928, well within safe range Co-Authored-By: Claude (global.anthropic.claude-opus-4-5-20251101-v1:0) <noreply@anthropic.com>
FredM67
added a commit
that referenced
this pull request
Jan 30, 2026
Add comprehensive test suite for the DC offset filter changes in PR #127: - 20 unit tests covering initialization, tracking, edge cases, precision - 5 comparison tests proving equivalence with old filter implementation - Documents int16_t overflow at ADC max (1023) - safe range is 0-1022 - Verifies new filter tracks same as old (within 5 ADC counts) - Documents slower time constant (~400 vs ~200 cycles) for better noise immunity Key findings: - Mathematical equivalence confirmed between old and new implementations - New filter has no explicit limits (vs old 412-612 range) - Normal 230V operation uses ADC ~96-928, well within safe range Co-Authored-By: Claude (global.anthropic.claude-opus-4-5-20251101-v1:0) <noreply@anthropic.com>
Add comprehensive test suite comparing old (right-aligned ADC) vs new (left-aligned ADC) implementations for power and RMS calculations. Key finding: With matching DC offset, implementations are MATHEMATICALLY IDENTICAL (<0.1% difference over full cycle). The ~2% difference seen with default settings comes from DC offset choice (511 vs 512), NOT from algorithm differences. Test coverage (19 tests total): - Basic power calculation (4 tests) - RMS voltage calculation (2 tests) - Full cycle comparison (3 tests) - Edge cases and precision (5 tests) - Extended tests with noise (5 tests): * 10,000 cycles long-term stability * ±2 LSB realistic ADC noise * ±5 LSB stress noise * Varying power factor (0°-90°) with noise * 50,000 cycles stress test (~17 min simulated) All tests confirm safe migration path from old to new implementation. Co-Authored-By: Claude (global.anthropic.claude-opus-4-5-20251101-v1:0) <noreply@anthropic.com>
Add 6 new tests simulating realistic solar/household scenarios: - Multiple current amplitudes (20-400, light to heavy load) - Sinusoidal load variation (5000 cycles) - Random load steps (appliance on/off, 10000 cycles) - Cloud shadow simulation (solar dynamics, 10000 cycles) - Daily solar profile (24-hour pattern, 8640 cycles) - Import/export transitions (bidirectional flow, 10000 cycles) All tests verify <1% difference between old and new implementations across varying current amplitudes and dynamic load conditions. Total test suite now covers 25 scenarios including noise and edge cases. Co-Authored-By: Claude (global.anthropic.claude-opus-4-5-20251101-v1:0) <noreply@anthropic.com>
Update realistic load variation tests to use full ADC range: - test_multiple_current_amplitudes: now tests 10 levels (20-505) - test_sinusoidal_load_variation: range 50-500 - test_random_load_steps: range 20-505 - test_cloud_shadow_simulation: clear sky at 500 - test_daily_solar_profile: peak at 500 Adjusted tolerances for edge cases: - 0.2% for random/cloud tests at high amplitudes - 3% for daily profile (includes many low-power night cycles) All 25 tests pass across full dynamic range. Co-Authored-By: Claude (global.anthropic.claude-opus-4-5-20251101-v1:0) <noreply@anthropic.com>
Add printf-based statistics output to all realistic load variation tests: - test_multiple_current_amplitudes: tabular output for 10 amplitude levels - test_sinusoidal_load_variation: cycles, range, and power comparison - test_random_load_steps: cycles, range, and power comparison - test_cloud_shadow_simulation: cycles, range, and power comparison - test_daily_solar_profile: cycles, range, and power comparison - test_import_export_transitions: cycles, range, and power comparison Also adds TestStats struct (unused but available for future use) and includes <cstdio> header for printf functionality. Co-Authored-By: Claude (global.anthropic.claude-opus-4-5-20251101-v1:0) <noreply@anthropic.com>
Bring the ADLAR fixed-point branch up to date with dev (which itself was just resynced with main after PR #138's squash-merge). Conflict resolutions: - mult_asm.h: kept dev's version. The assembly instruction sequences are byte-identical on both sides; dev's adds template parameters with is_same_v static_asserts documenting the required argument types. - processing.cpp: kept this branch's version. dev's two unique changes (the `if constexpr (lpf_gain != 0.0F && alpha != 0.0F)` guard and the adc_ctx_t linked-list ADC ISR) are already present here, with the ADLAR redesign layered on top. - test/embedded/test_mult_asm/test_main.cpp: kept this branch's much more thorough version, casting its bare int literals to the argument types dev's static_asserts require. Two int-promoted subtractions in the simulation loops needed an explicit int16_t cast as well. Also wire three suites into the CI matrix that had never run there: test_voltage_accumulation (embedded), test_dc_offset_filter and test_power_rms_comparison (native). These are exactly the tests that validate this branch's DC-offset and power/RMS rework. The timing benchmarks (test_mult_asm_perf) stay out, as wall-clock assertions are unreliable under Wokwi. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Whitespace only: clang-format 18 re-aligns the trailing-comment column of each group, and the static_cast<> additions shifted those groups. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The Wokwi CI harness runs with --fail-text "FAIL" and aborts on the first match anywhere in the serial log. The "Overflow Demo (should FAIL with >>8 for 20s)" banner tripped it, so the job failed even though all six Unity tests passed. Reword the banner and note the constraint inline so it is not reintroduced. No test logic changed. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Unity compiles TEST_ASSERT_GREATER_THAN_UINT64 into an unconditional "64-bit Support Disabled" failure on AVR, so test_20s_shift8_overflow could never pass. Compare against UINT32_MAX directly and assert on the bool instead; the uint64_t arithmetic itself works fine on AVR. Serial.print() is likewise 32-bit on AVR, so the accumulator was printed truncated (1809433306 instead of ~6.10e9, i.e. the true sum mod 2^32), which made the diagnostic output actively misleading. Add printU64(). The test's premise was correct: 20s at >>8 really does exceed UINT32_MAX, which is what makes the >>12 shift mandatory for periods above 10s. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Closed
processing.h still declared int16_t overloads of processCurrentRawSample()
and processVoltageRawSample(). Since the ADLAR rework those are defined
nowhere - the definitions in processing.cpp take uint16_t. It links today
only because ISR(ADC_vect) passes a uint16_t, an exact match; any caller
passing a signed value would silently bind the undeclared overload and fail
at link time. Remove them.
Also update processMinusHalfCycle()'s documentation, which still described
the pre-ADLAR clamped filter ("approximately one percent of the average
offset ... previous mains cycle", "remains within defined limits"). The
filter is now a free-running Q17.15 integrator with no min/max clamp, as
covered by test/native/test_dc_offset_filter.
Restore the file header @date/@copyright, which had regressed to 2021.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
These seven files carry no change other than an @Date bump applied by the old pre-commit hook, which stamped every staged file whether or not it had been edited. They add nothing to the ADLAR work and would conflict pointlessly with feature/remote-loads. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
FredM67
changed the title
This was referenced Oct 2, 2026
FredM67
added a commit
that referenced
this pull request
Oct 2, 2026
…board (#184) The universal 3phaseDiverter board (rev. 6.0 and later) is designed for the internal 1.1 V reference of the ATmega328P (buffered from AREF by an LMV321A), which the firmware must select. Until now the firmware always used AVCC, the reference of the old 3-phase board, so a new board measured wrongly. The change was on the unmerged new-pcb branch (cfe0682), written for the ISR before the circular channel list of #127. - config.h: PCB_VERSION, OLD (default, unchanged behaviour) or NEW; - processing.cpp: _ADMUX selects AVCC or the internal 1.1 V, and the first conversion now uses it too (it ran on the AREF setting before the ISR took over); - the startup summary shows the board; - READMEs (EN/FR): how to choose, and to recalibrate after a change; CHANGELOG. Checked: OLD gives the same ADMUX values as before (0x60 | channel) and passes the grid simulator scenarios; NEW gives 0xE0 | channel. Co-authored-by: Claude <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Pull request
Proposed changes
Implements both suggestions from #121 (thanks @florentbr).
1 — Circular linked list for ADC channel selection
The ISR's
switchover the six channels is replaced by a static circular list ofadc_ctx_t { next, index, admux }. Each entry carries theADMUXvalue for thefollowing channel, so the ISR just follows the pointer and writes
ADMUX— nodispatch, no bounds check.
2 — Left-aligned ADC conversion (
ADLAR = 1)ADMUXsetsADLAR, so raw samples arrive at ×64 scale (10 bits, left-alignedin 16). Rounding is a plain
rawSample | 32U.processVoltage()adds every
i_sampleVminusDCtouint32_t l_filterDC_V, andprocessMinusHalfCycle()takesl_filterDC_V >> 15. Each sample feeds back witha gain of 1/32768; the accumulator is never reset and needs no clamp.
>> 2, ×16) before multiplication, so both thepower and V² products stay in a 16×16→32 multiply and the result is shifted back
by 8.
Calibration is unchanged. The old path was ADC ×256 →
>> 2→ product ×4096 →>> 12→ ×1; the new one is ADC ×64 →>> 2→ product ×256 →>> 8→ ×1. Netscaling is identical, so existing
f_powerCal/f_voltageCalvalues carry overas-is — no user action on upgrade.
V² accumulator headroom.
processVoltage()picks its shift from the datalogperiod:
>> 8up to 10 s,>> 12above. At the default 5 s (1600 sample sets/s perphase) a full-scale sine accumulates ~1.0e9 into the
uint32_t, leaving ~4×headroom.
Tests
test/native/test_power_rms_comparison/— runs the old and new pipelines side byside over synthetic waveforms and asserts the power / Vrms results agree.
test/native/test_dc_offset_filter/— covers the Q17.15 integrator (settling,saturation, polarity at the rails).
test/embedded/test_voltage_accumulation/— verifies the accumulator does notoverflow across datalog periods and voltages.
test/embedded/test_mult_asm/— extended for the new operand ranges.All four are wired into the CI matrices in
.github/workflows/build.yml.Hardware validation
Bench-tested on commit 5d20297 with
f_powerCal 0.050000andf_voltageCal0.81510 / 0.81840 / 0.81950: V1 reads 234.4–234.8 V, free RAM 1611,
minSampleSets/MC 32(equal to the theoretical maximum of 1600 / 50, i.e. no sampleset dropped),
#ofSampleSets ~8015per 5 s period.Deferred
Storing
i_sampleVminusDCpre-decimated to 14 bits, to avoid shifting the voltagetwice per sample set (#121 comment).
Worth ~4 cycles (0.25 µs), but it makes the variable's scale depend on where you are
in the six-channel ISR interleave — ×64 until the DC filter is updated, ×16
afterwards — which a future reordering would break silently. Not taken while the ISR
has this much margin; revisit if timing ever gets tight.
Closes #121
Types of changes
Checklist