Skip to content

fix(evse.keba): apply SCALE_FACTOR_1 to Keba energy limit register - #3863

Open
sankalpsthakur wants to merge 6 commits into
OpenEMS:developfrom
sankalpsthakur:fix/3765-keba-energy-limit-scale-factor
Open

fix(evse.keba): apply SCALE_FACTOR_1 to Keba energy limit register#3863
sankalpsthakur wants to merge 6 commits into
OpenEMS:developfrom
sankalpsthakur:fix/3765-keba-energy-limit-scale-factor

Conversation

@sankalpsthakur

Copy link
Copy Markdown

Summary

The Keba EVSE energy-limit register (5010, SET_ENERGY_LIMIT) is documented in units of 10 Wh, but the write task mapped it with no scale converter — matching the // TODO Scalefactor for Unit: 10 Wh comment already left in the code at that exact line. As a result, setting a 1000 Wh energy limit wrote raw register value 1000 instead of 100, a 10x error.

SCALE_FACTOR_1 is already used elsewhere in this class for other 10 Wh-scaled registers, so no new converter is needed — this applies the existing one to register 5010 and removes the now-resolved TODO.

Validation

  • Patch applies cleanly against current develop.
  • Verified by direct inspection against ElementToChannelConverter's documented behavior: SCALE_FACTOR_1 converts raw register value 1 to channel value 10 (and the inverse on write), which is exactly the 10 Wh scaling the register requires.
  • Did not run the full Gradle build locally (disk space constraint in this environment) — relying on CI for the build/test matrix. Expected regression, callable from existing test patterns in this module: writing a channel value of 1000 Wh should produce raw register value 100.

Fixes #3765

AI/LLM disclosure

  • AI coding tools (including Grok and/or Codex agent-assisted editing) were used to help draft or modify code and this PR description.
  • I reviewed the complete change, understand the reasoning, and ran the reported local tests before submitting.
  • This submission is original work of authorship under the project CLA / contributor terms; AI output was not pasted unreviewed.

Register 5010 (SET_ENERGY_LIMIT) is documented in units of 10 Wh, but
the write task mapped it with no converter, so a channel value of
1000 Wh was written as raw 1000 instead of 100 - a 10x error.

SCALE_FACTOR_1 is the same converter already used elsewhere in this
class for 10 Wh-scaled registers; no new converter is needed.

Fixes OpenEMS#3765
@sfeilmeier

Copy link
Copy Markdown
Contributor

Thank you. Could you please fix the same issue also in EvcsKebaModbusImpl? Please also add a JUnit test in both EvcsKebaModbusImplTest and EvseChargePointKebaModbusImplTest

@sfeilmeier
sfeilmeier self-requested a review August 2, 2026 19:40

@sfeilmeier sfeilmeier left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

See above

@sankalpsthakur

Copy link
Copy Markdown
Author

See above

Applied the same SCALE_FACTOR_1 fix in EvcsKebaModbusImpl and added the requested JUnit coverage in both EvcsKebaModbusImplTest and EvseChargePointKebaModbusImplTest.

The PR is mergeable and CI is waiting for maintainer authorization. Ready for re-review.

@sankalpsthakur sankalpsthakur left a comment

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

All requested changes have been applied and verified, ready for re-review.

@sankalpsthakur

sankalpsthakur commented Aug 5, 2026

Copy link
Copy Markdown
Author

Hi @sfeilmeier, verified both fixes and tests are present on 7f6e748. Ready for re-review.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Keba P40 Energy Limit Scaling Bug

2 participants