fix: correct libbi charge target percentage calculation (resolves #727) - #749
Open
jtracey93 wants to merge 6 commits into
Open
fix: correct libbi charge target percentage calculation (resolves #727)#749jtracey93 wants to merge 6 commits into
jtracey93 wants to merge 6 commits into
Conversation
The TargetChargePercentSlider was dividing/multiplying by the full physical battery capacity (mbc), but the myenergi app uses the usable capacity which accounts for a hardware SoC reserve of ~9.8%. The usable capacity ratio is 92/102 (~0.9020) across all Libbi variants: - 10 kWh (mbc=10200): 9200 Wh usable (9200/10200 = 92/102) - 20 kWh (mbc=20400): 18400 Wh usable (18400/20400 = 92/102) Source: twonk/MyEnergi-App-Api docs + issue CJNE#727 field data. Changes: - Add LIBBI_USABLE_CAPACITY_FACTOR = 92 / 102 constant - Fix native_value: use usable capacity as divisor; return None (not 0) when charge_target is None so HA shows 'unknown' instead of a misleading 0% - Fix async_set_native_value: apply factor when converting % to Wh; use round() consistently instead of truncating int() - Add tests/fixtures/client_libbi.json (mbc=20400) - Add mock_libbi_set_charge_target fixture to conftest.py - Add TargetChargePercentSlider tests: read 100%/70%/no-creds, write 70% -> 12880 Wh, write 100% -> 18400 Wh Fixes CJNE#727 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Collaborator
|
Can you have a look at the failed test? |
- Add Libbi.refresh_extra mock to setup helper to prevent OAuth calls - Add client_fixture parameter to setup_mock_myenergi_config_entry - Move assertions inside PropertyMock context for read tests - Use real charge_target property for no-credentials test - Migrate test_sensor.py to use client_fixture parameter Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Author
|
@AJediIAm these should now be fixed |
Collaborator
Sorry, the notification got snowed under. |
Collaborator
|
@jtracey93 : are the failing tests caused by this PR or are they unrelated? |
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
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.
Summary
Fixes #727 —
number.myenergi_libbi_charge_targetshows a lower percentage than the myenergi app for the same setting (e.g. app shows 70%, HA shows ~63%).Root cause
TargetChargePercentSliderwas dividing/multiplying by the full physical battery capacity (mbc), but the myenergi app uses the usable capacity which accounts for a hardware SoC reserve of ~9.8%.The fix
The usable capacity ratio is 92/102 ≈ 0.9020 — consistent across all Libbi variants:
Sources: twonk/MyEnergi-App-Api docs + field data from issue #727.
Before fix (20 kWh battery, app set to 70%):
energyTarget = 12880Wh12.88 / 20.4 * 100 = 63%❌After fix:
12.88 / (20.4 * 0.9020) * 100 = 70%✅Changes
custom_components/myenergi/number.pyLIBBI_USABLE_CAPACITY_FACTOR = 92 / 102constant (with explanation comment)native_value: divide bybattery_size × factor; returnNone(not0) whencharge_target is Noneso HA correctly showsunknownwhen app credentials are absentasync_set_native_value: multiply bybattery_size * 1000 * factor; useround()consistentlytests/fixtures/client_libbi.json— new test fixture withmbc=20400tests/conftest.py— addmock_libbi_set_charge_targetfixturetests/test_number.py— 5 new regression tests:unknownstate ✅