Skip to content

GivTCP discharge target written every cycle even when unchanged (regression from #4492) #4517

Description

@chalfontchubby

Follow-up to #4421, reported by @mpartington and @gcoan in the comments there after PR #4492 (which fixed the spurious "REST failed to setExportTarget got 0" warnings) merged:

Maybe I need to raise a new issue. The fix pushed to main has stopped the faults, however it is writing a discharge target twice every 5 mins in the GivTCP logs. I think this is number.givtcp_ce2146g269_target_soc on my AC3, however this is NOT in the list of RTC safe registers. So burning through unnecessary writes. The number of writes during a 2 HR discharge, seems so unnecessary. I don't even see anything being changed. - @mpartington

@gcoan confirmed seeing the same.

Hypothesis

#4492 fixed the verification inside rest_setDischargeTarget() (inverter.py:3390) to check Control.Discharge_Target_SOC_1 first, since GivTCP updates that synchronously at write time, falling back to the old raw.invertor.discharge_target_soc_1 register only if Control doesn't have it - that register only refreshes on GivTCP's separate background self_run poll cycle, not synchronously with the write, which was the root cause of #4421.

That fix only touched the inside of rest_setDischargeTarget(). The caller that decides whether to write at all is separate, at inverter.py:2502-2512:

if "raw" in self.rest_data and "invertor" in self.rest_data["raw"] and "discharge_target_soc_1" in self.rest_data["raw"]["invertor"]:
    current = self.rest_data["raw"]["invertor"]["discharge_target_soc_1"]
    ...
    elif current != target_soc:
        self.rest_setDischargeTarget(target_soc)
    else:
        self.log("Inverter {} Current discharge target is already set to {}".format(self.id, current))

This still compares against the same slow raw.invertor.discharge_target_soc_1 field the #4492 fix moved away from as an unreliable primary signal. If that field doesn't reliably track the real applied value on some hardware (AC3/older Gen1 - there's been a running suspicion in #4421 that this register behaves oddly on that line), current never matches target_soc, so this branch fires and re-writes on every cycle - the else branch ("already set to X") never gets reached. Before #4492 that produced a visible warning each time; after #4492, the underlying write still fires every cycle, just silently now, since the low-level retry now succeeds quickly via Control.

If this holds up, the fix would be applying the same primary/fallback pattern (Control.Discharge_Target_SOC_1 first, raw.invertor fallback) at this call site too, not just inside rest_setDischargeTarget().

@mpartington @gcoan - does that match what you're seeing? In particular, if you can check GivTCP's own Control.Discharge_Target_SOC_1 value against raw.invertor.discharge_target_soc_1 around one of these repeated-write cycles, that would confirm whether the raw field is actually the one lagging/stuck.

Activity

  1. chalfontchubby commented on Aug 14, 2026

    @chalfontchubby
    CollaboratorAuthor

    Opened #4518 with a fix and regression test.

  2. mpartington commented on Aug 14, 2026

    @mpartington

    Thanks for raising this, I was going to wait until I could jump on the laptop.

    I will do a full analysis, but without looking through the code, I can't tell what 'control.discharge_target_soc_1' maps to. GEN 1 inverters have never had an option to set a discharge target, this simply does not exist. If you did want to do this, then the work around is to set the reserve value, but then I suspect it would just sit there, drawing from the grid one it reached target.

    The soc target entity is only applicable to grid charging. I think settings relevant for newer model inverters have wrongly been implemented on older models that do not have the compatibility.

  3. mpartington commented on Aug 14, 2026

    @mpartington

    So the updated apps.yaml (apps/predbat/config/apps.yaml) has this, therefore it should not be trying to set on my gen 1 inverter. Is this one of those situations where it isn't appropriately gated when using REST :

    Image Image

    Note that i didn't have these lines in my apps.yaml

  4. chalfontchubby commented on Aug 14, 2026

    @chalfontchubby
    CollaboratorAuthor

    @mpartington You're right, and thanks for digging into this - it's a different, earlier problem than the one this issue was originally about.

    Confirmed in the code: apps.yaml gates discharge_target_soc correctly for the non-REST path (comment there literally says "Only gen3 supports discharge target SoC, will be ignored if not present") - but the REST path (inverter.py, if self.rest_data and self.rest_v3:) has no equivalent gate. rest_v3 only checks the GivTCP software version (GivTCP_Version.startswith("3")), not the physical inverter's generation - so a Gen 1 inverter running GivTCP v3 middleware sails straight through and Predbat attempts a write your hardware has never supported.

    My fix in #4518 doesn't address this - it assumes the write is meaningful, just noisy/mistimed. Yours is a step earlier: the write shouldn't be attempted at all on your hardware.

    To scope a proper capability gate, I need to see what your REST API actually reports for a Gen 1 inverter. Could you share:

    • The full raw.invertor section of a rest_data dump (or a debug.yaml) from your system
    • Ideally also GivTCP's own response for whatever endpoint reports inverter model/generation, if you know it

    That'll tell us whether there's a reliable field to gate on (model name, firmware string, feature flag) versus needing a manual override setting.

  5. mpartington commented on Aug 14, 2026

    @mpartington

    Hi, attached what I think you are asking for. If I'm wrong, please let me know how to capture the data. Debug is taken with a forced export running

    REST Response.txt
    GIVTCP_Logs.log
    predbat_debug.yaml (1).txt
    predbat.log

  6. mpartington commented on Aug 14, 2026

    @mpartington

    Given we are now EOL, prehaps Gen 1 (Hybrid and AC3 ) could be identified from firmware (given no new version will ever be released, so no need to maintain the list)

    https://github.com/GivEnergy/givenergy-firmware-files?fbclid=IwY2xjawTry3JwZG9mBWV4dG4DYWVtAjEwAGJyaWQRMEdzM25Ta0k4OWVHeTRmVUNzcnRjBmFwcF9pZBAyMjIwMzkxNzg4MjAwODkyAAEeF4ijrFCurLYOMxYarJFZ1MRIgPZGB3QerokYEv0qVPFVMQBHA0nNFdw9Ub8_aem_UkQsyx4w4eXDRv39YO6CHA

    AC3 List

    Image

    Gen1 Hybrid list

    Image

    Gen 2 Hybrid list

    Image
  7. chalfontchubby commented on Aug 14, 2026

    @chalfontchubby
    CollaboratorAuthor

    @mpartington Thanks for the data - that settles it, and it's not what I expected.

    Your REST_Response.txt shows Control.Discharge_Target_SOC_1 is completely absent from Control on your setup, and raw.invertor.discharge_target_soc_1 reads 4 in that one snapshot - a perfectly plausible value, not garbage. So it's not that your hardware lacks the register entirely (your earlier hunch): your predbat.log shows it actually gets set correctly sometimes ("Current discharge target is already set to 8.0" / "4.0"), but mostly doesn't catch up in time, causing a write nearly every cycle - sometimes twice within one 5-minute window, matching what you reported. That's the original #4421 staleness mechanism, on hardware where there's no Control signal to fall back on at all.

    #4518 (just updated to reflect this) fixes the redundant-write problem for setups where Control.Discharge_Target_SOC_1 is populated - it won't change anything for you, since it falls through to the same flaky raw.invertor field either way. Not a regression, just not a fix for your case.

    Your firmware-based idea is the right direction given Gen 1 (Hybrid/AC3) is EOL and the list won't grow - but building and maintaining that gate is a separate, bigger piece of work than this PR, so I'm tracking it here rather than folding it in. Will pick it up as a follow-up.

  8. mpartington commented on Aug 14, 2026

    @mpartington

    Thats interesting, so are we saying the parameter exists on Gen1, but there is no way to change it? Therefore it will keep attempting this.

    As this is an 'unsafe' register i.e. one not stored in RAM, it would make sense to minimise the writes on all GEN versions. Is there ever a reason on a GIV system for it not to be set to 4?

  9. mpartington commented on Aug 14, 2026

    @mpartington

    A bit more information, the value changing between 4 and 8 and back to 4 is actually my reserve value (number.givtcp_ce2146g269_battery_power_reserve). I have an automation to artificially raise, to ensure the predbat planned discharge doesn't empty my batteries before 23.30 i.e. allows margin to any anomalies in SOC readings. I revert to 4% when the batteries hit 30% SoC.

    Image
  10. gcoan commented on Aug 14, 2026

    @gcoan
    Collaborator

    My understanding is that the discharge target SoC doesn't exist on the older inverters at all, the only floor limit that is used is the reserve, so shouldn't even be trying to write to it on our inverters.

    I don't have it configured in my apps.yaml.

    Firmware is one option to try to work out what inverter type predbat is talking to, the firmware version is in defined ranges, D0.2* for AC3 (and D0.5* for beta versions), D0.4* for gen 1 hybrid (and D0.1* for beta), etc.

    Alternative and simpler to look at inverter type its Ac for AC3 and Hybrid_gen1 on my Gen 1, and presumably similar for gen 2?

    @chalfontchubby do you need a dump of the REST response for my Gen 1 hybrid?

  11. mpartington commented on Aug 15, 2026

    @mpartington

    I have probably misunderstood the fix. I was hoping (although not a full fix) that it would limit the write attempts. However it's trying to write every 5 mins.

    Still unsure if this is actually clocking up EPROM writes, or a false positive in the GivTCP unsafe write count

  12. chalfontchubby commented on Aug 15, 2026

    @chalfontchubby
    CollaboratorAuthor

    (a change to)4518 fixes the redundant-write problem for setups where Control.Discharge_Target_SOC_1 is populated - it won't change anything for you, since it falls through to the same flaky raw.invertor field either way. Not a regression, just not a fix for your case.

    Sorry - I'd intended that to mean it would change nothing in your case. I've not found time to look at this one again since - kinda busy.

  13. chalfontchubby commented on Aug 15, 2026

    @chalfontchubby
    CollaboratorAuthor

    @mpartington Dug into your REST_Response.txt/GIVTCP_Logs.log further, and there's a genuine theory worth testing here - opened a draft PR: #4541.

    Your Control.Enable_Charge_Target reads "disable", and it's never appeared in your GivTCP log at all (no enableChargeTarget write attempts, successful or otherwise) - matching that #4518's fix never touches this code path, since it's on the charge-target side, not discharge. adjust_battery_target() already calls rest_enableChargeTarget(True) before writing a charge target ("without it the inverter ignores the target") - it was never mirrored for the discharge target write, which #4541 does.

    I want to be upfront that this is a theory, not a confirmed fix - it's correlational (compared your data against a working sample from an unrelated report where Enable_Charge_Target is "enable" and Discharge_Target_SOC_1 works fine), not something I can verify without your hardware. I also can't tell you whether these writes actually hit EEPROM specifically, or some other persisted-but-not-wear-prone store - that's genuinely unknown without GivEnergy's own register documentation, which doesn't exist anywhere I have access to (and with them having gone under, isn't coming).

    I did try to make sure this can't make things worse if the theory is wrong: if enabling the register itself fails (as it would if your inverter genuinely doesn't support it at all, not just never having been asked), the discharge-target write is now skipped rather than also attempted - so worst case you're retrying one REST endpoint a cycle instead of two, same as before, not more.

    Would you be willing to test the branch (fix/givtcp-discharge-target-enable-gate) and watch your GivTCP log for a while? Specifically interested in:

    1. Does Control.Enable_Charge_Target actually flip to "enable" and stay there, or does it also fail/revert?
    2. If it does enable, does the discharge target write stop repeating every cycle?
    3. If enabling itself fails, do you see the new "Unable to enable charge target, discharge target not written" log line instead of the old repeated write attempts?

    Any of those three outcomes tells us something useful either way.

  14. mpartington commented on Aug 15, 2026

    @mpartington

    Hi thanks for looking into this.
    Control.Enable_Charge_Target works.fine,.it is only required though when the soc grid charge target is less than 100%. As my off peak rate is significantly less than my export rate, there is never a scenario that makes sense to charge to less than 100%. Therefore this remains off by choice, rather than a system limitations.

    I am.100% sure this will.make.no.difference.to.the non existent register for the discharge target.

    The only fix is stopping Predbat attempting to write to this register for inverters earlier than gen 3

  15. 7 remaining items

  16. mpartington commented on Aug 16, 2026

    @mpartington

    @chalfontchubby Hi Rik, thanks agian for all your work on this. Happy to confirm that the latest patch #4543 has fixed the issue for me.

    I've exported for a period and seen no error messages, or 'unsafe writes' recorded by GivTCP, or any unexpected behaviour. Looks good to merge into main

    predbat (3).log

    Image Image
  17. mpartington commented on Aug 16, 2026

    @mpartington

    EDIT - see it's already been merged to main

  18. gcoan commented on Aug 16, 2026

    @gcoan
    Collaborator

    @chalfontchubby I've upgraded to the version on main

    Export this morning, can see the discharge target being set every 5 minutes:
    image

    And doing a quick export test, I can see that I don't see messages about setting discharge target on my AC3 inverter (inverter I) but I am still seeing them from my two gen 1 hybrid's (inverters G and H).

    image

    The Gen 1 and 2 hybrid's also don't support the discharge target register, only the gen 3, so the test at line 2934 of inverter.py will need to be extended to check for the hybrid gen 1 or 2 strings please

    image

  19. chalfontchubby commented on Aug 16, 2026

    @chalfontchubby
    CollaboratorAuthor

    @gcoan Thanks for testing the merged fix live and catching this - opened #4551 extending it to Hybrid Gen1/Gen2, based on your findings and the GivEnergy firmware archive's generational split. Hybrid_gen1 is confirmed from your own two inverters; Hybrid_gen2 is inferred from the same archive, not independently confirmed yet, so flagged as such in the code in case that turns out to need adjusting. Would appreciate a live test the same way if you get the chance.

  20. gcoan commented on Aug 16, 2026

    @gcoan
    Collaborator

    Perfect Rik, I copied inverter.py from your patch fork into my predbat, set a force export, and there's nothing in the givtcp log indicating predbat attempted to write to the discharge target on any of the three inverters, just set the export slot times and set mode to timed export

    Image

    predbat does appear to be setting Battery Pause mode unnecessarily for the export, it was set to PauseCharge (because I was in Freeze Export mode to stop the inverters cross charging), and for an export as long as battery pause mode is not set to PauseDischarge or PauseBoth then the battery will discharge. At the start of the export Predbat sets battery pause mode to Disabled which it doesn't need to, because it then has to set it back to PauseCharge when going back to FreezeExport at the end of the export
    I'll have a look at the other times pause mode is changed and see if there is a broader optimisation pattern that could be considered, but this isn't a big issue

  21. springfall2008 commented on Aug 17, 2026

    @springfall2008
    Owner

    I think I released this fix last night

  22. gcoan commented on Aug 17, 2026

    @gcoan
    Collaborator

    I think I released this fix last night

    Hi @springfall2008 there was a first fix for AC3 which is merged, there's then a subsequent one #4451 for hybrid inverters that isn't yet merged

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions