domoticz / domoticz

Open source Home Automation System
http://www.domoticz.com
GNU General Public License v3.0
3.44k stars 1.12k forks source link

V2020.2 : Incorrect 'Today' readings for 'Energy read: Computed' General Electric sensors with negative instant value #4736

Closed mvdklip closed 3 years ago

mvdklip commented 3 years ago

When updating a General Electric (kWh) sensor which has been set to 'Energy read: Computed' with a negative instant value (wattage), the 'Today' value displayed in the web interface is correctly negative but not the correct value. The Today value seems to 'jump' back to 0 regularly.

I have created a test plugin which sends the same wattage to two sensors, one being a negative value and the other a positive. It updates every 5 seconds and lets Domoticz compute the counter. The counters seems to be fine if you look at the logging after a bit of time:

2021-03-19 14:04:41.756 (testit - Negative wattage) Updating device from 0:'-3600;-1278.0' to have values 0:'-3600;0'.
2021-03-19 14:04:41.765 (testit - Positive wattage) Updating device from 0:'3600;1278.0' to have values 0:'3600;0'.

Both counters are the same except for the sign. However if you look at the web interface something's not right:

Negative wattage

The Today value for the negative wattage is way off. If you keep looking at this value you can see it jump sometimes.

Tested on the latest stable version (2020.2). I am unable to test this on the latest beta right now.

Update 1: this problem has been confused with another 'toggling' problem introduced by a fix for #4733. Although that is not the same problem as described above a lot of the comments below are about this toggling problem.

Update 2: this has been tested on beta build 13111 and the problem doesn't seem to manifest itself there so it seems limited to the latest stable.

mvdklip commented 3 years ago
"""
<plugin key="testit" name="testit" author="mvdklip" version="1.0.0">
    <description>
        <h2>Test Plugin</h2><br/>
    </description>
    <params>
        <param field="Mode3" label="Query interval" width="75px" required="true">
            <options>
                <option label="5 sec" value="1" default="true"/>
                <option label="15 sec" value="3"/>
                <option label="30 sec" value="6"/>
                <option label="1 min" value="12"/>
                <option label="3 min" value="36"/>
                <option label="5 min" value="60"/>
                <option label="10 min" value="120"/>
            </options>
        </param>
    </params>
</plugin>
"""

import Domoticz

class BasePlugin:
    enabled = False
    lastPolled = 0

    def __init__(self):
        return

    def onStart(self):
        Domoticz.Debug("onStart called")
        Domoticz.Debugging(1)

        if len(Devices) < 1:
            Domoticz.Device(Name="Negative wattage", Unit=1, TypeName='kWh', Options={'EnergyMeterMode':'1'}).Create()
        if len(Devices) < 2:
            Domoticz.Device(Name="Positive wattage", Unit=2, TypeName='kWh', Options={'EnergyMeterMode':'1'}).Create()

        DumpConfigToLog()

        Domoticz.Heartbeat(5)

    def onStop(self):
        Domoticz.Debug("onStop called")

    def onHeartbeat(self):
        Domoticz.Debug("onHeartbeat called %d" % self.lastPolled)

        if self.lastPolled == 0:
            Devices[1].Update(nValue=0, sValue="-3600;0")
            Devices[2].Update(nValue=0, sValue="3600;0")

        self.lastPolled += 1
        self.lastPolled %= int(Parameters["Mode3"])

global _plugin
_plugin = BasePlugin()

def onStart():
    global _plugin
    _plugin.onStart()

def onStop():
    global _plugin
    _plugin.onStop()

def onHeartbeat():
    global _plugin
    _plugin.onHeartbeat()

# Generic helper functions
def DumpConfigToLog():
    for x in Parameters:
        if Parameters[x] != "":
            Domoticz.Debug("'" + x + "':'" + str(Parameters[x]) + "'")
    Domoticz.Debug("Device count: " + str(len(Devices)))
    for x in Devices:
        Domoticz.Debug("Device:           " + str(x) + " - " + str(Devices[x]))
        Domoticz.Debug("Device ID:       '" + str(Devices[x].ID) + "'")
        Domoticz.Debug("Device Name:     '" + Devices[x].Name + "'")
        Domoticz.Debug("Device nValue:    " + str(Devices[x].nValue))
        Domoticz.Debug("Device sValue:   '" + Devices[x].sValue + "'")
        Domoticz.Debug("Device LastLevel: " + str(Devices[x].LastLevel))
    return
FireWizard52 commented 3 years ago

I can confirm related strange behaviour. See for the details: https://www.domoticz.com/forum/viewtopic.php?f=6&t=35839

kovainfo commented 3 years ago

all kWh sensors stopped working

rwaaren commented 3 years ago

all kWh sensors stopped working

This statement is nor true nor does it bring us it any closer to finding a cause for this issue. Please add some details like your domoticz version, settings of your device and how they are updated.

I am on build 13091 and my kWh sensors updated by dzVents and set as 'from device' are working as before.

image

image

image

FireWizard52 commented 3 years ago

Hello @rwaaren

I can imagine, that it is working in your case, as you use "From device". I' m on 13091 as well, but all my devices with a configuration "Computed" are failing. The devices with "From device" are okay.

The issue is introduced in 13088, yesterday. See also: https://github.com/domoticz/domoticz/issues/4733

And also: https://www.domoticz.com/forum/viewtopic.php?f=6&t=35839.

For me, it is pretty clear, but I am not able to reverse the change.

rwaaren commented 3 years ago

@FireWizard52, I already was aware that the issue was with the devices set to computed. The reason that I commented was to prevent someone from investigating what I already noticed: that devices set to "From device" where not hampered by this issue. Without my comment developers looking into this might have been set on the wrong foot by the statement of @kovainfo that All KWh devices stopped working.

kovainfo commented 3 years ago

Ok, all MY kWh devices stopped working 🤷🏻‍♂️

now I have rolled back to the previous version of Domoticz, which worked successfully for me ( Build Hash: 066f3f261-modified Build Date: 2021-03-08 18:27:50 )

The error manifested itself like this: At first, the "Today" counter showed a negative value, and then the values simply stopped being displayed.

I use only two types of kWh devices: a custom device that is updated through Domoticz API and Xiaomi kWh devices from sockets, etc.

And, yes, my devices havе the option: "Computed"

mvdklip commented 3 years ago

Please comment in #4733 if your meters stopped working / started toggling. That behaviour seems to be introduced by a supposed fix for #4733.

Although I'm the reporter of both issues, they are not related. This issue is about something else which manifests itself in the latest stable (2020.2.11995).

kovainfo commented 3 years ago

Domoticz 2020.2 (build 13058)

Снимок экрана 2021-03-20 в 14 02 28

but last beta:

Снимок экрана 2021-03-20 в 14 24 30
devrosx commented 3 years ago

same problem here with latest beta Return type works but usage type is empty... (im using computed energy read) for moment it works but then is somehow erased...

gizmocuz commented 3 years ago

There was a issue in the computed energy routine. The crash had been solved, but introduced another issue. A new beta has been made and the problem should be solved now. Please close the issue if that's the case ;)

FireWizard52 commented 3 years ago

I loaded the new beta (version 13092), but another issue has been introduced. If you set the type to "Return" and "Energy read" has been set to "Computed" the devices switch back to default, but the selector field (Usage/Return) is empty.

To reproduce:

  1. Set type to "Return"
  2. Set "Energy read" to "Computed"

[Added] Also the values are toggling still. So it is not solved, but it is getting worse.

devrosx commented 3 years ago

Yes today count is still empty and values are still toggling with latest build....

gizmocuz commented 3 years ago

Things are getting confused now... Two related issues does also not help. If this is a issue that was introduced by 2020.2 stable, yeah it won't be solved by any beta version until someone makes a PR to solve it

kovainfo commented 3 years ago
Снимок экрана 2021-03-21 в 13 20 48

:(((((

kovainfo commented 3 years ago

build 13058 works fine for me, but now I accidentally erased it and now I don't know what to do (

FireWizard52 commented 3 years ago

@gizmocuz

I already said so, that things are getting worse, you call it confused. It has nothing to do with stable. In my opinion it has been introduced in 13088. I did not install that version, but noticed the issue for the first time in 13090 and also 13091. 13092 did not solve it, but introduced, beside the issue with the toggle of the value (see screenshot of @kovainfo), also the issue with the setting of Usage/Return.

Currenly I have set the "Energy read" to "From device" and that gives at least the instant value, but of course the accumulated value is missing.

This gives a stable situation.

kovainfo commented 3 years ago

@gizmocuz I installed the latest working beta a week ago (March 13-14). for me this version was stable (build 13058).

I think the changes that were made to the kWh device last week and caused the indicated problems

Снимок экрана 2021-03-21 в 13 58 50
FireWizard52 commented 3 years ago

The problem is still there.

To reproduce:

Set the configuration as follows:

Screenshot_Energy Computed1

With the Type as return. Close this window Inject a value (e.g. by MQTT/Node Red) The icon changes to standard (as for Usage) and the selector field is empty as on the picture. Configure "Return" Inject once again it is okay, but change another the first one changes again.

Change the second one, the first one toggles again.

It looks very random, but there is a relationship.

So still not possible to use "Computed" and "Return"

gizmocuz commented 3 years ago

@FireWizard52 , there are two issues that are maybe related to each other. The other issue (first created) was about a crash because of a empty initial sValue. This crash should hopefully be fixed now Confusing because the author of that thread (mvdklip) is also reporting here.

I will now try to replicate the issue (just using normal JSON to update the meter)

gizmocuz commented 3 years ago

I've added a general kwh sensor, set the type to return and energy read=computer:

image

Using the following JSON call to update the sensor

http://127.0.0.1:8080/json.htm?type=command&param=udevice&idx=7828&nvalue=0&svalue=722;3556

image

Every time I update the widget updates and does not toggle, or flip, or changes it's type

gizmocuz commented 3 years ago

image

devrosx commented 3 years ago

on my side its still toggling im using MQQT like this for example

2021-03-22 08:21:39.922 MQTT: Topic: domoticz/in, Message: {"idx":940,"command":"udevice","svalue":"199;0"}

gizmocuz commented 3 years ago

Maybe you/we could test with the JSON API call first ? I am not familiar with this type of device, maybe it is because behind the semicolon there is a zero (POWER;0) ... I do some more testing as well

gizmocuz commented 3 years ago

It seems to work perfectly with

http://127.0.0.1:8080/json.htm?type=command&param=udevice&idx=7828&nvalue=0&svalue=1625;0

Could you also test with using the JSON call to update the sensor ?

If this keeps working normally, we know the internal Domoticz code is good, and the issue might be related to MQTT

mvdklip commented 3 years ago

@FireWizard52 , there are two issues that are maybe related to each other. The other issue (first created) was about a crash because of a empty initial sValue. This crash should hopefully be fixed now Confusing because the author of that thread (mvdklip) is also reporting here.

I am sorry for the confusion but the two issues are totally unrelated. Unfortunately this issue was 'hijacked' by people reporting the side effects of the initial fix for #4733.

What shall I do? Close this issue and open a new one for this problem? Because, as said, this is a different issue with negative wattages.

gizmocuz commented 3 years ago

@mvdklip , Best to check if the other issue is now solved and is yes close that issue

Let's keep this issue open for this problem

FireWizard52 commented 3 years ago

Hello @gizmocuz

I used to send the Solar data with MQTT to Domoticz. As you indicated, that you don't have any problems with http JSON call, I rebuilt my Node Red "Function" node in order to send http instead of mqtt. It doesn't make any difference.

The data is toggling.

See: Screenshot_Problem Solar-return-computed1

As you can see, some has the "Usage" icon and some still has the "Return" icon. In case of the "Usage" icon, the selector field is empty. The "Today" counter is not counting at all, probably because it is reset, when it is empty. Is it reset?

In order to avoid any coincidental interference with other programs, I created a new sensor on my test machine and sent every 2 seconds a JSON message, copied from the one you provided and changed only the IDX,.

The result is exactly the same. It toggles every 2 seconds.

Screenshot_Problem Solar-return-computed2

Screenshot_Problem Solar-return-computed3

Conclusion so far: It doesn't make any difference, whether I use MQTT or HTTP. Problem start, as soon as I use "Energy read" setting as "Computed". Problem started after upgrade from 13084 to 13090 and in my opinion it is caused by changes introduced in 13088.

I do not know, what to check/test further.If you need more info, let me know.

Regards

gizmocuz commented 3 years ago

@FireWizard52 , could you take a step back please and

How does that workout ?

FireWizard52 commented 3 years ago

I did one more test.

The toggle behavior is independent of the value in "svalue=1625;0". I doesn't matter what the figure after 1625 is. 0 is okay, but any other figure shows the same. However if you omit the 0, the screen stops to toggle and shows "no value", as shown in the last picture. Even if you use a minus value it continue to toggle but the "Today" shows a minus value.

Another thing, worthwhile to mention, is that you see the value on the "Today" counter, being similar as the last value in the string. In my opinion this value should be completely ignored, as the setting for "Energy read" has been set to "Computed" and the value should be based on the first (Power) value (1625), calculated by Domoticz. Perhaps this indicated a direction.

FireWizard52 commented 3 years ago

Add a dummy hardware if you don't have one already Add a dummy kwh meter (instant+counter) Edit the new sensor, set it to return and computed Use a browser to send the JSON calls to update the device (like my URL above ... with a different idx value)

How does that workout ?

I already had "Dummy hardware" and a Dummy kWh meter (instant + counter) The sensor was already prepared (Return and Computed)

As I previous used a small test flow in Node Red, injecting the time stamp every 2 seconds and sending the same URL, my expectations were not very high. And indeed, it toggles after every transmission. Except, if I use the browser, I get the confirmation that:

status "OK"
title "Update Device

Anything else, I can do?

gizmocuz commented 3 years ago

@FireWizard52 , it works out that you do not use MQTT or any special way to update the sensor, and have a new test sensor to 'test' against If you open two browser tabs (one with the view on the test widget), and one where you enter the json call, you should see the widget update And every 2 seconds does not make sense, try once every 10 seconds/minute

And yes, you need to supply the last value (even if that is zero), the sValue is Instance;Counter as documented in the API

FireWizard52 commented 3 years ago

@gizmocuz

To be clear.

For the test I do not use MQTT, but for the test I use the JSON call, provided by you, earlier today.

it works out that you do not use MQTT or any special way to update the sensor, and have a new test sensor to 'test' against

Yes I do/have a dedicated "test" sensor on a dedicated Raspberry Pi with a dedicated Domoticz, version 13104

If you open two browser tabs (one with the view on the test widget), and one where you enter the json call, you should see the widget update

I did and saw the toggle.

And every 2 seconds does not make sense, try once every 10 seconds/minute

I know and agree, For normal use it is useless, however if you want to test something you can configure it to do it faster in order to get the final result earlier. To monitor the widget and to observe, whether it toggles or not, a waiting time of 1 minute is not very smart.

And yes, you need to supply the last value (even if that is zero), the sValue is Instance;Counter as documented in the API

I know, but in an earlier post, you wrote:

I am not familiar with this type of device, maybe it is because behind the semicolon there is a zero (POWER;0) ...

That was for me the trigger to play with this value. In my case, it has been zero, all time, but, as said, I noticed that if I insert a non-zero value, this value is displayed in the widget, even if the "Energy read" is set to "Computed" This is strange, according to the API description:

(if you choose as type "Energy read : Computed", this is just a "dummy" counter, not updatable because Domoticz will calculate this internally based on the (previous/current) POWER value)

I would not expected to see that value, as I expect that Domoticz will calculate the value itself based on the received POWER.

Regards

gizmocuz commented 3 years ago

Could anyone else also try with my instructions above ? I am not able to reproduce this, making it hard to solve.

I created a bash script that updates the sensor every two seconds, and there are no issues at all. kWh is calculated, correct usage is displayed on the top right, sensor type stays on return/computed

#!/usr/bin/env bash

while :
do
        echo "Press [CTRL+C] to stop.."
        wget -q -O /dev/null 'http://192.168.0.123:8080/json.htm?type=command&param=udevice&idx=7828&nvalue=0&svalue=1625;0'
        sleep 2
        wget -q -O /dev/null 'http://192.168.0.123:8080/json.htm?type=command&param=udevice&idx=7828&nvalue=0&svalue=225;0'
        sleep 2
        wget -q -O /dev/null 'http://192.168.0.123:8080/json.htm?type=command&param=udevice&idx=7828&nvalue=0&svalue=123;0'
        sleep 2
done

You need to change the ip address and idx value to the correct one

devrosx commented 3 years ago

Screenshot 2021-03-23 at 10 15 51

https://user-images.githubusercontent.com/13789041/112122754-e2cd0200-8bc0-11eb-9d16-5fd59a7e28a5.mov

Could anyone else also try with my instructions above ? I am not able to reproduce this, making it hard to solve.

I created a bash script that updates the sensor every two seconds, and there are no issues at all. kWh is calculated, correct usage is displayed on the top right, sensor type stays on return/computed

#!/usr/bin/env bash

while :
do
        echo "Press [CTRL+C] to stop.."
        wget -q -O /dev/null 'http://192.168.0.123:8080/json.htm?type=command&param=udevice&idx=7828&nvalue=0&svalue=1625;0'
        sleep 2
        wget -q -O /dev/null 'http://192.168.0.123:8080/json.htm?type=command&param=udevice&idx=7828&nvalue=0&svalue=225;0'
        sleep 2
        wget -q -O /dev/null 'http://192.168.0.123:8080/json.htm?type=command&param=udevice&idx=7828&nvalue=0&svalue=123;0'
        sleep 2
done

You need to change the ip address and idx value to the correct one

hmm weird you bash script works fine on new created IDX device, but when i change idx in your script to my current device its broken and togling...

FireWizard52 commented 3 years ago

@gizmocuz

I tested your provided bash script as well.

I created a new virtual kWh device on the Raspberry Pi with Domoticz 13104. By default is the virtual sensor configures as "Usage" and "From device" While the script runs, I reconfigured the virtual sensor to "Return" and "Computed".

Immediately the behavior of the sensor changed to what is shown on the video of @devrosx

As in @devrosx video could be seen that his sensor has been set to "Usage", I tried that as well. This doesn't make any difference.

One interesting thing, I did not note before, because I tested only with one value that does not change is that it skips a value.

From your script, you should expect:

1625W, followed by 225W, followed by 123W and then the script start all over again. The result is 1625, empty, 123, empty, 225, empty 1625. So it skips a message Exactly, what you see in the movie.

This bash sript is similar to what I did yesterday, when I injected every 2 seconds a URL to Domoticz from Node Red

So, sorry it is still there and it doesn't make any difference, whether it is a bash script, a http message, or a mqtt message. It is toggling.

rwaaren commented 3 years ago

Using script

#!/usr/bin/env bash

testID=1298

sqlite3 -noheader /opt/domoticz/domoticz.db "select * from devicestatus where id = $testID" 

while :
do
        echo "Press [CTRL+C] to stop.."
        wget -q -O /dev/null "http://192.168.192.115:8084/json.htm?type=command&param=udevice&idx=$testID&nvalue=0&svalue=1625;0"
        sqlite3 -noheader /opt/domoticz/domoticz.db "select * from devicestatus where id = $testID" 
        sleep 2
        wget -q -O /dev/null "http://192.168.192.115:8084/json.htm?type=command&param=udevice&idx=$testID&nvalue=0&svalue=225;0"
        sqlite3 -noheader /opt/domoticz/domoticz.db "select * from devicestatus where id = $testID" 
        sleep 2
        wget -q -O /dev/null "http://192.168.192.115:8084/json.htm?type=command&param=udevice&idx=$testID&nvalue=0&svalue=123;0"
        sqlite3 -noheader /opt/domoticz/domoticz.db "select * from devicestatus where id = $testID" 
        sleep 2
done

I get this

1298|3|00083298|1|issue4736|1|243|29|0|0|12|255|0|1625;0|2021-03-23 12:43:54|865|0.0|1.0|0.0|1.0|||0|0|0||EnergyMeterMode:MQ==|
▒|2021-03-23 12:43:56|865|0.0|1.0|0.0|1.0|||0|0|0||EnergyMeterMode:MQ==|
1298|3|00083298|1|issue4736|1|243|29|0|0|12|255|0|123;0|2021-03-23 12:43:58|865|0.0|1.0|0.0|1.0|||0|0|0||EnergyMeterMode:MQ==|
Press [CTRL+C] to stop..
▒|2021-03-23 12:44:00|865|0.0|1.0|0.0|1.0|||0|0|0||EnergyMeterMode:MQ==|
1298|3|00083298|1|issue4736|1|243|29|0|0|12|255|0|225;0|2021-03-23 12:44:02|865|0.0|1.0|0.0|1.0|||0|0|0||EnergyMeterMode:MQ==|

So something funny ends up in the database.. and likely in the device.

Before image

After

image

rwaaren commented 3 years ago

Using

sqlite3 -noheader /opt/domoticz/domoticz.db "select svalue from devicestatus where id = $testID" | hexdump -c | head -1

produces

0000000   1   6   2   5   ;   0  \n
0000000   0  \r 312   s 017 177  \n
0000000   1   2   3   ;   0  \n
Press [CTRL+C] to stop..
0000000   0  \r 312   s 017 177  \n
0000000   2   2   5   ;   0  \n
0000000   0  \r 312   s 017 177  \n
Press [CTRL+C] to stop..
0000000   1   6   2   5   ;   0  \n
0000000   0  \r 312   s 017 177  \n
0000000   1   2   3   ;   0  \n
Press [CTRL+C] to stop..
0000000   0  \r 312   s 017 177  \n
0000000   2   2   5   ;   0  \n
0000000   0  \r 312   s 017 177  \n
gizmocuz commented 3 years ago

Those are indeed strange values !! Maybe this does not happen in debug mode...

mvdklip commented 3 years ago

@mvdklip , Best to check if the other issue is now solved and is yes close that issue

Let's keep this issue open for this problem

I will keep this open, but please note there are three issues really:

  1. Domoticz crashing as documented in #4733. I believe the crash itself is fixed although I can't really check because I'm using the stable version of Domoticz. Instead I found a workaround which prevents the crash for me.
  2. Strange values / toggling as a result of the fix for #4733. I did not report that because I'm not experiencing and (cannot experience) those problems because I'm running stable. Discussing those problems IMO belongs in #4733 or a separate issue.
  3. This issue which is about feeding negative instant values to a kWh meter set to computed. This has nothing to do with point 1 or 2 but is simply another bug present in (at least) 2020.2.11995.

NOFI but @FireWizard52 unintentionally hijacked this issue thinking that I was describing the toggling problems, which I was not.

mvdklip commented 3 years ago

Now carry on discussing the toggling problems here if you want, because it does sound quite nasty and is messing up databases. I will just observe and try to fix the situation with the mixup of two separate issues later. :-D

rwaaren commented 3 years ago

I activated debug logging and added some extra debuglines in SQLHelper.cpp

I noticed that for every update from the test bash script the code of line 4873-4880 (see below) is executed twice for the same device id. btw. 1298 is the device idx of my test device used in the test script.

_log.Debug(DEBUG_NORM, "SQLH UpdateValueInt line 4873: id = %d ", ulID);
            result = safe_query(
                "UPDATE DeviceStatus SET SignalLevel=%d, BatteryLevel=%d, nValue=%d, sValue='%q', LastUpdate='%04d-%02d-%02d %02d:%02d:%02d' "
                "WHERE (ID = %" PRIu64 ")",
                signallevel, batterylevel,
                nValue, sValue,
                ltime.tm_year + 1900, ltime.tm_mon + 1, ltime.tm_mday, ltime.tm_hour, ltime.tm_min, ltime.tm_sec,
                ulID);
2021-03-23 22:00:08.297  Debug: SQLH UpdateValueInt line 4873: id = 1298
2021-03-23 22:00:10.315  Debug: SQLH UpdateValueInt line 4873: id = 1298
2021-03-23 22:00:10.414  Debug: SQLH UpdateValueInt issue4736 HwID:3  DevID:00083298 Type:243  sType:29 nValue:0 sValue:123;0
2021-03-23 22:00:12.434  Debug: SQLH UpdateValueInt line 4873: id = 1298
2021-03-23 22:00:14.453  Debug: SQLH UpdateValueInt line 4873: id = 1298
2021-03-23 22:00:14.453  Debug: SQLH UpdateValueInt issue4736 HwID:3  DevID:00083298 Type:243  sType:29 nValue:0 sValue:225;0
2021-03-23 22:00:16.471  Debug: SQLH UpdateValueInt line 4873: id = 1298
2021-03-23 22:00:18.489  Debug: SQLH UpdateValueInt line 4873: id = 1298
2021-03-23 22:00:18.489  Debug: SQLH UpdateValueInt issue4736 HwID:3  DevID:00083298 Type:243  sType:29 nValue:0 sValue:1625;0
2021-03-23 22:00:20.778  Debug: SQLH UpdateValueInt line 4873: id = 1298
lwalbert commented 3 years ago

Hi @gizmocuz

Maybe I have solved the toggling issue, atleast it looks OK on my system now. from line 4740 just below localtime_r(&now, &ltime); replace the code in SQLHelper.cpp its related to the #4733

//Update to fix toggling of object and domoticz crashing
                std::string wpart = result[0][5];
                std::vector<std::string> parts;
                StringSplit(sValue, ";", parts);
                StringSplit(wpart, ";", parts);
                //Commit: If Option 1: energy is computed as usage*time
                //Default is option 0, read from device
                if (options["EnergyMeterMode"] == "1" && (parts.size() == 2) && devType == pTypeGeneral && subType == sTypeKwh)
                {
                        struct tm ntime;
                        double interval;
                        char sCompValue[100];
                        std::string sLastUpdate = result[0][6];
                        time_t lutime;
                        ParseSQLdatetime(lutime, ntime, sLastUpdate, ltime.tm_isdst);

                        interval = difftime(now, lutime);

                                double nEnergy = std::stof(parts[0]) * interval / 3600 + std::stof(parts[1]);
                                sprintf(sCompValue, "%s;%.1f", parts[0].c_str(), nEnergy);
                                old_sValue = sCompValue;
                }

br lwalbert

rwaaren commented 3 years ago

@lwalbert,

👍 your patch works OK on my system. Tested with the @gizmocuz bash testscript.

lwalbert commented 3 years ago

hi @rwaaren Thats good to hear, Im still checking the calculation part, so there might be some adjustments.

gizmocuz commented 3 years ago

@lwalbert , some questions remarks:

'wpart/parts' is only used when

So this could be inside the 'if' check, do not check yet for parts.size() == 2 because there could be faulty data instead, add a second 'if' below the previous one to check if the parts equals 2 and use the below code between the two 'if' s

            std::vector<std::string> parts;
            StringSplit(sValue, ";", parts);
            StringSplit(wpart, ";", parts);

You use twice 'SplitString' , I thing this one is not needed

            StringSplit(sValue, ";", parts);

I will do some testing as well

gizmocuz commented 3 years ago

I think this was the fix:

old_sValue = sCompValue;

gizmocuz commented 3 years ago

I'm not sure if the kWh counter is increasing with the above patched code, you are storing the result in 'old_sValue', but that is not used anymore so it does not increment.. Only the Power value is changing in the widget

gizmocuz commented 3 years ago

This is the original code from weeks ago:

    if (options["EnergyMeterMode"] == "1" && devType == pTypeGeneral && subType == sTypeKwh)
    {
        std::vector<std::string> parts;
        struct tm ntime;
        double interval;
        float nEnergy;
        char sCompValue[100];
        std::string sLastUpdate = result[0][6];
        time_t lutime;
        ParseSQLdatetime(lutime, ntime, sLastUpdate, ltime.tm_isdst);

        interval = difftime(now, lutime);
        StringSplit(result[0][5], ";", parts);
        nEnergy = static_cast<float>(strtof(parts[0].c_str(), nullptr) * interval / 3600
                         + strtof(parts[1].c_str(), nullptr)); // Rob: whats happening here... strtof ?
        StringSplit(sValue, ";", parts);
        sprintf(sCompValue, "%s;%.1f", parts[0].c_str(), nEnergy);
        sValue = sCompValue;
    }

And this caused a crash if sValue did not consisted of two values (separated by a semicolon)

I made the protection patch again and this is the result:

    if (options["EnergyMeterMode"] == "1" && devType == pTypeGeneral && subType == sTypeKwh)
    {
        std::vector<std::string> parts;
        struct tm ntime;
        double interval;
        float nEnergy;
        char sCompValue[100];
        std::string sLastUpdate = result[0][6];
        time_t lutime;
        ParseSQLdatetime(lutime, ntime, sLastUpdate, ltime.tm_isdst);

        interval = difftime(now, lutime);
        StringSplit(result[0][5], ";", parts);
        if (parts.size() == 2)
        {
            nEnergy = static_cast<float>(std::stof(parts[0]) * interval / 3600 + std::stof(parts[1]));
            StringSplit(sValue, ";", parts);
            if (!parts.empty())
            {
                sprintf(sCompValue, "%s;%.1f", parts[0].c_str(), nEnergy);
                sValue = sCompValue;

            }
        }
    }

I think it's correct ... should be correct

It is available in beta 13111 ...

FireWizard52 commented 3 years ago

Unfortunately the problem has not been solved. Energy read has been set to "Computed" . Upgraded to version 13111 and tested with the previously provided Bash script. It still toggles The strange thing is that in addition also the setting (Usage/Return) of another kWh sensor is changed to an "empty" value. That happens quite often. This setting is not stable.