Skip to content

Add optional EcoFlow CDC telemetry to usbhid-ups - #3569

Open
user01010111 wants to merge 5 commits into
networkupstools:masterfrom
user01010111:feature/ecoflow-cdc-telemetry
Open

Add optional EcoFlow CDC telemetry to usbhid-ups#3569
user01010111 wants to merge 5 commits into
networkupstools:masterfrom
user01010111:feature/ecoflow-cdc-telemetry

Conversation

@user01010111

Copy link
Copy Markdown

Summary

  • Add optional lifecycle hooks for HID subdrivers which need a companion transport.
  • Add a read-only EcoFlow CDC ACM companion selected with ecoflow_cdc_port.
  • Expose standard power, load, frequency, temperature, capacity and outlet readings through existing NUT names.
  • Keep EcoFlow-specific readings under experimental.ecoflow.*.
  • Document the configuration and add captured-frame protocol tests.

Rationale

The EcoFlow RIVER 3 Plus HID interface provides the data needed for UPS monitoring and shutdown, but it does not provide live power and temperature telemetry. The same USB device exposes richer read-only data through a CDC ACM interface.

This change keeps HID authoritative and enriches the same NUT device instead of creating a second partial UPS. CDC support is opt-in. A missing or failed CDC connection reconnects independently and never marks the HID data stale.

The protocol framing and field work builds on the public r3pcomms project and was independently verified against a RIVER 3 Plus.

Related to #2735.

Safety

  • Only the known telemetry request is implemented.
  • No device-control commands are present.
  • Response preamble, declared length, CRC and sequence number are validated.
  • Malformed segments are rejected.
  • CDC failures are rate-limited and isolated from HID state.
  • The feature does nothing unless ecoflow_cdc_port is configured.

Testing

  • make -j2 check: 7 passed, 0 failed.
  • Protocol test with a captured, serial-redacted response.
  • AddressSanitizer and UndefinedBehaviorSanitizer protocol test.
  • usbhid-ups build with serial support enabled.
  • usbhid-ups build with serial support disabled.
  • Live RIVER 3 Plus test: HID status, charge and runtime remained present while CDC added power, load, frequency, temperature, capacity and outlet data.
  • Live missing-port test: CDC reported reconnect.trying while HID remained responsive and continued reporting status, charge and runtime.

Contributor note

I used an AI coding assistant during implementation. I reviewed the resulting changes and validated them with the tests and hardware checks listed above.

The commit includes the required DCO sign-off.

Signed-off-by: user01010111 <lapses.50.booster@icloud.com>
@github-actions

github-actions Bot commented Aug 13, 2026

Copy link
Copy Markdown

A ZIP file with standard source tarball and another tarball with pre-built docs for commit 55bf78d is temporarily available: NUT-tarballs-PR-3569.zip.

@AppVeyorBot

Copy link
Copy Markdown

@AppVeyorBot

Copy link
Copy Markdown

Build nut 2.8.5.5051-master failed (commit 637bf29682 by @)

user01010111 added 2 commits August 13, 2026 18:06
Signed-off-by: user01010111 <lapses.50.booster@icloud.com>
Signed-off-by: user01010111 <lapses.50.booster@icloud.com>
@AppVeyorBot

Copy link
Copy Markdown

Signed-off-by: user01010111 <lapses.50.booster@icloud.com>
@AppVeyorBot

Copy link
Copy Markdown

@AppVeyorBot

Copy link
Copy Markdown

Build nut 2.8.5.5054-master completed (commit 6506ebb503 by @)

@AppVeyorBot

Copy link
Copy Markdown

Build nut 2.8.5.5054-master completed (commit 6506ebb503 by @)

@jimklimov jimklimov left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Quite solid, thanks. Commented on a couple of stylistic nits, but neither is something that requires an immediate code change - more as hints for future contributions.

"system temperature parses");
check(metrics.has_battery_temperature && metrics.battery_temperature == 28,
"battery temperature parses");
check(metrics.has_output_power && fabs(metrics.output_power - 258.768) < 0.01,

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One minor suggestion, not a blocker: in include/nut_float.h we have #define f_equal(x, y) ( fabsf((float)(x) - (float)(y)) <= FLT_EPSILON ) (and other similar macros) to make code like this simpler to read. In a test this is probably not even worth fixing, so just to let you know if this pattern would appear elsewhere.

Comment thread drivers/usbhid-ups.c
upsdebugx(1, "upsdrv_cleanup...");

if (subdriver != NULL && subdriver->aux != NULL &&
subdriver->aux->cleanup != NULL) {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A little style nit, I think applies to various spots in both of your PRs, and is loosely documented at docs/developers.txt section Indenting with tabs vs. spaces (maybe should be rewritten more explicitly, so I'll update) - we find it easier to read indented code with a visually clear boundary of where the opening clause ends and a scoped code section starts. So for multi-line clauses, the final parentheses tend to go onto their own line with original indent level:

if (A &&
    B) {
    ...
}

becomes more like

if (A
&&  B
) {
    ...
}

or

if (A
 && B
) {
    ...
}

A single-line clause remains indented as it was:

if (A && B) {
    ...
}

I think there's no immediate need to change this PR, unless you plan more work on the feature. There are similar "offenders" elsewhere in the code base and I hope would be eventually covered with some code styling automation, although our particular set of tweaks about convenient C mark-up does not seem common (for .editorconfig, .clang-format et al) so such automation is still waiting for its maker :)

@jimklimov jimklimov added enhancement USB serial port EcoFlow Issues and PRs about EcoFlow River 3, Delta 3 UPS series AI For good or bad, machine tools are upon us. Humans are still the responsible ones. labels Aug 13, 2026
@jimklimov jimklimov added this to the 2.8.6 milestone Aug 13, 2026

@jimklimov jimklimov left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, my earlier review missed an aspect that does need a change: bump driver versions (usbhid-ups.c itself, and the *-hid.c subdrivers, due to structure format change), at least if nothing else bumped them since v2.8.5 release.

I wonder also if ecoflow-cdc{,-protocol}.{c,h} might stand as a driver code base on its own merit, or is strictly meant to be part of usbhid-ups? Maybe introducing a structured naming for the new aux tweaks should be useful (e.g. we have ecoflow-hid for usbhid-ups, so AUX tweaks could be ecoflow-hid-aux-something - WDYT?)

@user01010111

Copy link
Copy Markdown
Author

Thanks for the review. I’ve updated the branch to bump usbhid-ups and all HID subdriver versions, rename the runtime companion to ecoflow-hid-aux-cdc, and use f_equal() for the exact floating-point test comparisons.

I retained ecoflow-cdc-protocol because it is the transport-independent parser, and kept the existing configuration and state names for compatibility. The updated code passes the Clang warning check, unit tests, serial and non-serial builds, and distribution checks locally.

Signed-off-by: user01010111 <lapses.50.booster@icloud.com>
@user01010111
user01010111 force-pushed the feature/ecoflow-cdc-telemetry branch from eb4e00f to 55bf78d Compare August 13, 2026 11:30
@user01010111
user01010111 marked this pull request as ready for review August 13, 2026 11:41
@AppVeyorBot

Copy link
Copy Markdown

Build nut 2.8.5.5057-master completed (commit d83200915e by @)

@AppVeyorBot

Copy link
Copy Markdown

Build nut 2.8.5.5058-master completed (commit adc52e30d2 by @)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI For good or bad, machine tools are upon us. Humans are still the responsible ones. EcoFlow Issues and PRs about EcoFlow River 3, Delta 3 UPS series enhancement serial port USB

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants