Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

no yield per day after sunset with HMS-Series, HM is ok #1397

Closed
kr0815 opened this issue Oct 1, 2023 · 69 comments
Closed

no yield per day after sunset with HMS-Series, HM is ok #1397

kr0815 opened this issue Oct 1, 2023 · 69 comments
Labels
bug Something isn't working

Comments

@kr0815
Copy link

kr0815 commented Oct 1, 2023

What happened?

I have 2 OpenDTU-Installations

  • one with 4 HM-1200 Inverters
  • the other one with 3 HMS-2000 Inverters

After sunset, i see on the HM-1200 inverters the yield by day
In the HMS-Installation, YPD shows zero after sunset

To Reproduce Bug

what shall i say :-)

Expected Behavior

see YPD for both installations

Install Method

Pre-Compiled binary from GitHub

What git-hash/version of OpenDTU?

v23.9.18

Relevant log/trace output

No response

Anything else?

No response

@kr0815 kr0815 added the bug Something isn't working label Oct 1, 2023
@AloisKlingler
Copy link

since I disabled the "poll inverter at night" this did not occur any more for me.
see #979

@kr0815
Copy link
Author

kr0815 commented Oct 3, 2023

thanks, i will give it a try :-)

I don´t understand - how to even poll the inverters at night - they don´t send anything when there is no input?

@huste511
Copy link

huste511 commented Oct 3, 2023

I also have several HMS here and have never been able to see the daily yield after sunset.

I also deactivated nighttime polling.

Since OpenDTU can determine the day itself, smaller values ​​from the same inverter should be ignored - it would be even better to cumulate them if necessary.

@kr0815
Copy link
Author

kr0815 commented Oct 4, 2023

I changed the settings for "poll inverter at night" - still strange , now another of the 3 HMS shows a yield per day, the others zero - musst be really a bug?

The HM-Inverters show all as expected

@huste511
Copy link

huste511 commented Oct 4, 2023

Not really a bug in openDTU but an area where improvements could be made.

The HMS switches off at some point at dusk. If it gets a little brighter at some point, it starts again, usually doesn't provide any energy, but for it it's still a "new day" and it and delivers 0 values...

@tbnobody
Copy link
Owner

tbnobody commented Oct 4, 2023

I am currently bad out of order. But please change the eventlog. Is it possible that your hms inverters reboot at evening? In this case the daily counter is resettet. Happens when e.g. at the evening the wether is cloudy and at the end the sun still comes out a bit to power the inverter again. Hm and hms might react differently here.

@kr0815
Copy link
Author

kr0815 commented Oct 4, 2023

--> tbnobody - get well soon :-)

What do you mean with changing the event log?

You are right, it could definetly be possible they reboot, show zero because of that event

@tbnobody
Copy link
Owner

tbnobody commented Oct 4, 2023

I mean, have look at the event log. Sry. Brain is mud.

In the event log should see the last startup time of the inverter.

@kr0815
Copy link
Author

kr0815 commented Oct 4, 2023

sorry if it sounds stupid, but where to see the event log? :-)

info - console?

@tbnobody
Copy link
Owner

tbnobody commented Oct 4, 2023

Ne. It's in the live view. I top of each in inverter are several buttons. One is for the event log

@kr0815
Copy link
Author

kr0815 commented Oct 4, 2023

strange:

Inverter 1 - YPD 0 - inverter start 19:17
Inverter 2 - YPD 0 - inverter start 07:09
Inverter 3 - YPD 11,9 kWh - inverter start 07:09

The HM all say Inverter Start around 7:14

@tbnobody
Copy link
Owner

tbnobody commented Oct 4, 2023

Maybe hm require less power to keep them operational. But there is already a first walkaroumd for this issue. Will have a detailed look and modify maybe the on or other thing. Bit this feature will come in if I am back.

@kr0815
Copy link
Author

kr0815 commented Oct 4, 2023

Take your time :-)

It´s fascianting, - what i observe, HM are generally better regarding output
not much, but it´s noticible

@broth-itk
Copy link
Contributor

broth-itk commented Oct 5, 2023

Duplicate of #1268 ?

I proposed a fix to cache the YieldDay values.

The problem can be observed when the interver restarts

  • in the evening
  • if panel is covered and disconnected for maintenance
  • intentionally on interter restart

@kr0815
Copy link
Author

kr0815 commented Oct 6, 2023

i guess there is another problem / bug:

See the red Message on the HM-1200, letzte Aktualisierung
With the HMS-2000 it is not red

HMS-2000
HM-1200

@tbnobody
Copy link
Owner

tbnobody commented Oct 6, 2023

Grey means that you've disabled polling at night. And that it is currently night.

@Lineflyer
Copy link

Lineflyer commented Nov 11, 2023

I am currently bad out of order. But please change the eventlog. Is it possible that your hms inverters reboot at evening? In this case the daily counter is resettet. Happens when e.g. at the evening the wether is cloudy and at the end the sun still comes out a bit to power the inverter again. Hm and hms might react differently here.

I do also have this exact issue with my HMS.
I thought the "Zero daily yield at midnight" feature was the foreseen fix/workaround for this problem...but seems not. What is the benefit of this feature then?

For my understanding:
Is this value provided by the HMS or calculated within the DTU?
If its calculated in the DTU it should be an easy fix, if the value comes from the HMS it would need to be buffered and added within one day by the DTU.

@pimisen
Copy link

pimisen commented Nov 21, 2023

Same issue here. 2 HM. No issues. One HMS YPD gone after sunset

@GM-Jokemaster
Copy link

Same here HMS800 T2. (Newest Version) v23.11.16
Reset to 0 on the same Day. At Power lost at the OpenDTU (Reset at Midnight is active)
Also a Friend of mine same HMS same result.
Please try to Store the yield per Day and if NTC Time tells same Day = Add the new (now low) vlaue to the Last before "Power lost"?
TY !!

@fred777
Copy link

fred777 commented Nov 30, 2023

IMO, the difference between HM and HMS is, that the HMS will start to communicate much earlier than the HM.

The electronics of both inverters are powered by solar and as soon as there is enough power, they will start feeding to AC.

For the HM this will be also the time when it starts communicating.

But not so with the HMS. It will already communicate as soon as there is enough DC power.

Attached are two images from a HM700 with 2 panels and HMS2000 with 3 panels on a winter's day with solar panels covered with snow.

The purple line is AC power, the other lines are DC power from the solar panels.

image
image

@Lineflyer
Copy link

Lineflyer commented Nov 30, 2023

Yes, its basically that the chance of the HMS to stop transmitting due to sunset and then getting alive again for a few seconds short after seems significantly higher than with the HM series.
This effect IMHO causes the YTD reset....and that is why I asked for clarification how the "reset ad midnight" works. I thought it should exactly prevent this problem. Otherwise it should be enhanced to cover this problem of multiple "restarts" of transmission during a day.

@broth-itk
Copy link
Contributor

Yes, its basically that the chance of the HMS to stop transmitting due to sunset and then getting alive again for a few seconds short after seems significantly higher than with the HM series. This effect IMHO causes the YTD reset....and that is why I asked for clarification how the "reset ad midnight" works. I thought it should exactly prevent this problem. Otherwise it should be enhanced to cover this problem of multiple "restarts" of transmission during a day.

Agree, I noted this problem long ago.
This is why I implemented caching of the YTD value (see above comment from Oct 5 and pull request).

At daychange the offset value needs to get resetted. Due to new implementations this could maybe done elsewhere.
Haven't looked at the code since then.

@tbnobody
Copy link
Owner

tbnobody commented Dec 1, 2023

It's already in my dev branch. But I am currently out of order. Therefor. Please keep patient.

@broth-itk
Copy link
Contributor

broth-itk commented Dec 1, 2023

@tbnobody: Patience is not a problem for me. Thanks for your great work!

Gute Besserung!

@pimisen
Copy link

pimisen commented Dec 1, 2023

@tbnobody Gute Besserung!
Maybe that is of use for anybody: HMS and HM seem to be different (But i guess you know that already ;-) )
Home Assistant Yield curves look kinda different:
Screenshot 2023-12-01 091704

@Lineflyer
Copy link

It's already in my dev branch. But I am currently out of order. Therefor. Please keep patient.

Thanks for the update and all the best!

@Lineflyer
Copy link

@pimisen
Yes, this visualization shows the "bug". The production is reset to zero on the HMS while it just stays stable on the HM and only resets on the following sunrise. The proposed fix should make your HMS curve look like the others.

tbnobody added a commit that referenced this issue Dec 16, 2023
@tbnobody
Copy link
Owner

This should be fixed with 1de3b48

You can enable this feature per inverter.

@chris299
Copy link

@tbnobody I just flashed the firmware.zip. I'll report how it will work today afternoon if the inverter restarts....

@fred777
Copy link

fred777 commented Dec 18, 2023

But wait - why do you keep setting _lastYieldDay[static_cast<uint8_t>(c)] = 0; in StatisticsParser::endAppendFragment() ?

Because the offset is applied in the line above.

Yes, but is that cumulative? To me it looks like the offset is just stored and when _lastYieldDay is then being reset to 0 on the next line, then on next call of endAppendFragment this code path will not be hit anymore and _lastYieldDay will be updated with current (much lower or even zero) yield value from the inverter in the else-path which is not what we want I guess.

@tbnobody
Copy link
Owner

Yes, but is that cumulative?

Yes kind off.... The offset is applied in the getChannelFieldValue function. Therefor the _lastYieldDay contains the current raw value from the inverter + the current offset.

image

@broth-itk
Copy link
Contributor

@chris299 You can just restart the inverter from GUI. This will have the same effect.

@tbnobody
Copy link
Owner

you can just restart the inverter from GUI.

Great idea!! I've just tested it.... and it seems to work as expected.

@fred777
Copy link

fred777 commented Dec 18, 2023

Yes, but is that cumulative?

Yes kind off.... The offset is applied in the getChannelFieldValue function. Therefor the _lastYieldDay contains the current raw value from the inverter + the current offset.

Ah, I see. Good - the day hasn't ended yet, so I'll revert my change now and see what happens after inverter restart.

@chris299
Copy link

@chris299 You can just restart the inverter from GUI. This will have the same effect.

@tbnobody just restarted the inverter and it seems to work :-)

@fred777
Copy link

fred777 commented Dec 18, 2023

I did not restart my inverter but waited for the sunset instead so that the inverter would restart by itself.

=> seems to work too 👍

image

@tbnobody
Copy link
Owner

The fix is included in todays version: https://github.com/tbnobody/OpenDTU/releases/tag/v23.12.18

I am going to close this issue. Please feel free to open it again if there are still issues.

@chris299
Copy link

chris299 commented Dec 19, 2023

@tbnobody looks like something else went wrong now:
the yield was not reset at midnight, although configured and it shows a dot instead of a comma.
looks like it was set to 65535-yield of yesterday....
(I'm still on the testing version from yesterday, not the 23.12.18 yet)
grafik

grafik

@tbnobody
Copy link
Owner

If you compile yourself, you could try this one:

diff --git a/lib/Hoymiles/src/Hoymiles.cpp b/lib/Hoymiles/src/Hoymiles.cpp
index 4cd3ef0..e7b3d26 100644
--- a/lib/Hoymiles/src/Hoymiles.cpp
+++ b/lib/Hoymiles/src/Hoymiles.cpp
@@ -133,10 +133,12 @@ void HoymilesClass::loop()
             if (currentWeekDay != lastWeekDay) {

                 for (auto& inv : _inverters) {
+                    // Have to reset the offets first, otherwise it will
+                    // Substract the offset from zero which leads to a high value
+                    inv->Statistics()->resetYieldDayCorrection();
                     if (inv->getZeroYieldDayOnMidnight()) {
                         inv->Statistics()->zeroDailyData();
                     }
-                    inv->Statistics()->resetYieldDayCorrection();
                 }

                 lastWeekDay = currentWeekDay;

@tbnobody tbnobody reopened this Dec 19, 2023
@chris299
Copy link

If you compile yourself, you could try this one:

@tbnobody unfortunately I'm not able to compile it myself....

BTW: Thank you very much for your effort and commitment! 👍 🙏

@tbnobody
Copy link
Owner

I am pretty sure that fixes the issue. will be included in todays release.

@broth-itk
Copy link
Contributor

broth-itk commented Dec 19, 2023

I'm running the latest version. A smaller inverter had 15Wh daily yield. After manual restart, it value drops to zero:

image

My understanding is that the day yield should be 15Wh (from cache, offset) + new Wh value from freshly restarted inverter.
Can I provide more information?

@tbnobody
Copy link
Owner

Can I provide more information?

Please tell me your version number. "latest" is not enough.
Please also provide the inverter config of this specific inverter.

@broth-itk
Copy link
Contributor

@tbnobody: OpenDTU v23.12.18

I've just noticed a new setting "Tagesertragskorrektur", wasn't aware about that.

As far as I was concerned, the code looks to work as designed - thanks!

@stefan123t
Copy link

@kr0815 @chris299 @Lineflyer could you test again with OpenDTU v23.12.18/19 ?
According to @broth-itk the code has been fixed and this issue could probably be closed if you report back too.

@fred777
Copy link

fred777 commented Dec 20, 2023

I just tested with the patch from #1397 (comment) applied to v23.12.18 and built from source.

Daily yield was perfectly reset to zero in the morning and started to grow on sunrise.

@chris299
Copy link

23.12.19 worked fine for me so far @stefan123t

@kr0815
Copy link
Author

kr0815 commented Dec 20, 2023

i tried with V19 - rebootet the inverter twice - looks like it works :-)

@stefan123t
Copy link

@tbnobody suggest to close this issue as fixed in 23.12.18/19.

@Lineflyer
Copy link

Lineflyer commented Dec 21, 2023

Yes, it also works with my HMS-1600 without further issues now!

Thanks @tbnobody

helgeerbe added a commit to hoylabs/OpenDTU-OnBattery that referenced this issue Dec 27, 2023
* Optimize Sun data calculation

* Remove not required enum

* Split config struct into different sub structs

* Feature: Allow configuration of LWT QoS

* Made resetreason methods static

* Feature: Implement offset cache for "YieldDay"

Thanks to @broth-itk for the idea!
Fix: #1258 tbnobody#1397

* Add Esp32-Stick-PoE-A

* remove broken LilyGO_T_ETH_POE config, use device profile instead

* Feature: High resolution Icon and PWA (Progressive Web App) functionality

Fix: #1289

* webapp: Update dependencies

* Initialize TaskScheduler

* Migrate SunPosition to TaskScheduler

* Migrate Datastore to TaskScheduler

* Migrate MqttHandleInverterTotal to TaskSchedule

* Migrate MqttHandleHass to TaskScheduler

* Migrate MqttHandleDtu to TaskScheduler

* Migrate MqttHandleInverter to TaskScheduler

* Migrate LedSingle to TaskScheduler

* Migrate NetworkSettings to TaskScheduler

* Migrate InverterSettings to TaskScheduler

* Migrate MessageOutput to TaskScheduler

* Migrate Display_Graphic to TaskScheduler

* Migrate WebApi to TaskScheduler

* Split InverterSettings into multiple tasks

* Calculate SunPosition only every 5 seconds

* Split LedSingle into multiple tasks

* Upgrade espMqttClient from 1.4.5 to 1.5.0

* Doc: Correct amount of MPP-Tracker

* Added HMT-1600-4T and HMT-1800-4T to DevInfoParser

Fix tbnobody#1524

* Adjusted inverter names for HMS-1600/1800/2000-4T

* Add channel count to description of detected inverter type (DevInfoParser)

* Adjust device web api endpoint for dynamic led count

* Feature: Added ability to change the brightness of the LEDs

Based on the idea of @moritzlerch with several modifications like pwmTable and structure

* webapp: Update dependencies

* Update olikraus/U8g2 from 2.35.7 to 2.35.8

* Remove not required onWebsocketEvent

* Remove code nesting

* Introduce several const statements

* Remove not required AsyncEventSource

* Doc: Added byte specification to each command

* Feature: Added basic Grid Profile parser which shows the used profile and version

Other values are still outstanding.

* Optimize AlarmLogParser to save memory

* Add libfrozen to project to create constexpr maps

* Feature: First version of GridProfile Parser which shows all values contained in the profile.

* webapp: Update dependencies

* Apply better variable names

* Remove not required casts

* Add additional compiler flags to prevent errors

* Add const statement to several variables

* Replace NULL by nullptr

* Update bblanchon/ArduinoJson from 6.21.3 to 6.21.4

* Add const keyword to method parameters

* Add const keyword to methods

* Use references instead of pointers whenver possible

* Adjust member variable names in MqttSettings

* Adjust member variable names in NetworkSettings

* webapp: Update timezone database to latest version

* webapp: Beautify and unify form footers

* Feature: Allow setting of an inverter limit of 0% and 0W

Thanks to @madmartin in #1270

* Feature: Allow links in device profiles

These links will be shown on the hardware settings page.

* Doc: Added hint regarding HMS-xxxx-xT-NA inverters

* Feature: Added DeviceProfile for CASmo-DTU

Based on tbnobody#1565

* Upgrade actions/upload-artifact from v3 to v4

* Upgrade actions/download-artifact from v3 to v4

* webapp: add app.js.gz

* Gridprofileparser: Added latest known values

Thanks to @stefan123t and @noone2k

* webapp: Fix lint errors

* Feature: Add DTU to Home Assistant Auto Discovery

This is based on PR 1365 from @CFenner with several fixes and optimizations

* Fix: Remove debug output as it floods the console

* Fix: Gridprofileparser: Add additional error handling if profile is unknown

* webapp: add app.js.gz

* Fix: Offset cache for "YieldDay" did not work correctly

* webapp: update dependencies

* webapp: add app.js.gz

* Fix: yarn.lock was outdated

* Fix: yarn build error

* Fix: Reset Yield day correction in combination with Zero Yield Day on Midnight lead to wrong values.

* Fix: Allow negative values in GridProfileParser

* Correct variable name

* Fix tbnobody#1579: Static IP in Ethernet mode did not work correctly

* Feature: Added diagram to display

This is based on the idea of @Henrik-Ingenieur and was discussed in tbnobody#1504

* webapp: update dependencies

* webapp: add app.js.gz

---------

Co-authored-by: Thomas Basler <[email protected]>
Co-authored-by: Pierre Kancir <[email protected]>
Copy link

github-actions bot commented Apr 2, 2024

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new discussion or issue for related concerns.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 2, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests