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

json update of Domoticz does not work anymore. #2660

Closed
11 tasks
berbergh opened this issue Oct 13, 2019 · 38 comments · Fixed by #2667
Closed
11 tasks

json update of Domoticz does not work anymore. #2660

berbergh opened this issue Oct 13, 2019 · 38 comments · Fixed by #2667

Comments

@berbergh
Copy link

Checklist

  • [x ] The title of this issue is "to the point" and descriptive.
  • [x ] This issue describes what is happening.
  • This issue describes what components are affected (e.g. name of plugin/controller)
  • This issue describes how to reproduce it.
  • This issue describes when it was introduced (when known) and what version is now showing the problem.

I have...

  • searched the issue tracker or the forum for a similar issue. (include links when applicable)
  • entered a system description using "Copy info to clipboard" on the sysinfo page. (when possible)
  • entered the full filename of the used version (e.g. ESP_Easy_mega-20181001_test_ESP8266_4096_VCC.bin )
  • given a list of active plugins or controllers when applicable.
  • filled out all applicable fields below.

Steps already tried...

  • [x ] Tried a clean install (empty .bin files are included in the ZIP)
  • Tested previous/other build (mention which one already tested)
  • Tested on other node to make sure hardware isn't defective.
  • Verified if the problem is limited to a single plugin/controller

If you self compile, please state this and PLEASE try to ONLY REPORT ISSUES WITH OFFICIAL BUILDS!

Summarize of the problem/feature request

Build:⋄ | 20104 - Mega
System Libraries:⋄ | ESP82xx Core 2.6.0-dev stage, NONOS SDK 2.2.2-dev(38a443e), LWIP: 2.1.2 PUYA support
Git Build:⋄ | mega-20191003
Plugins:⋄ | 78 [Normal] [Testing]
Build Md5: | 34624f5fdcf92673bd2d8b2cf9fefbf

I try to update a dummy Air Quality device in Domoticz using a json syntax.
Rules sends:
sendtohttp myurl,8080,/json.htm?type=command&param=udevice&idx=663&nvalue=[CO2#PPM]
In the log it looks like:
sendtohttp myurl,8080,/json.htm?type=command¶m=udevice&idx=663&nvalue=390

The effect is that when I enter:
myurl:8080/json.htm?type=command&param=udevice&idx=663&nvalue=[CO2#PPM]
in a browser, I get an update in Domoticz.
With the sentence in Rules, I don't.
It used to work before.

Expected behavior

YOUR TEXT GOES HERE

Actual behavior

YOUR TEXT GOES HERE

Steps to reproduce

System configuration

Hardware:

ESP Easy version:

ESP Easy settings/screenshots:

Rules or log data


@TD-er
Copy link
Member

TD-er commented Oct 13, 2019

Hmm, that's a strange replacement of characters:
&para into

Do you have any task or taskvalue containing para in the name?

@berbergh
Copy link
Author

berbergh commented Oct 13, 2019

No I don't.
It also happens to other Rules lines, like:
SendToHTTP myurl,8080,/json.htm?type=command&param=switchlight&idx=531&switchcmd=Toggle
becomes:
SendToHTTP myurl,8080,/json.htm?type=command¶m=switchlight&idx=531&switchcmd=Toggle
But that one seems to work okay.

@TD-er
Copy link
Member

TD-er commented Oct 14, 2019

Can you test by simply having the last part of that senttohttp sent to log using the command LogEntry ?
See the documentation - Command Reference

@berbergh
Copy link
Author

berbergh commented Oct 14, 2019

LogEntry shows this:
LogEntry,/json.htm?type=command¶m=udevice&idx=663&nvalue=467&svalue=0

@TD-er
Copy link
Member

TD-er commented Oct 14, 2019

So at least it is consistent there.

@uzi18
Copy link
Contributor

uzi18 commented Oct 15, 2019

try this:
SendToHTTP myurl,8080,/json.htm?type=command& param=switchlight&idx=531&switchcmd=Toggle

@TD-er
Copy link
Member

TD-er commented Oct 15, 2019

@uzi18 Just curious.
Why do you think that would work?
Either the sendtohttp command does interpret it as an extra parameter (don't think it will since there is no extra parameter defined) or it will change the space into %20

@uzi18
Copy link
Contributor

uzi18 commented Oct 15, 2019

just a trick, to avoid conversion for &para, if convert to %20 it should work on domoticz server

@TD-er
Copy link
Member

TD-er commented Oct 15, 2019

In the logentry it does convert it to ...& para....

I was just having a look at the code and this one seems strange to me:

bool conditionMatch(const String& check) {
int posStart, posEnd;
char compare = findCompareCondition(check, posStart, posEnd);
float Value1 = 0;
float Value2 = 0;
if (compare > ' ') {
String tmpCheck1 = check.substring(0, posStart);
String tmpCheck2 = check.substring(posEnd);
if (!isFloat(tmpCheck1) || !isFloat(tmpCheck2)) {
Value1 = timeStringToSeconds(tmpCheck1);
Value2 = timeStringToSeconds(tmpCheck2);
} else {
Value1 = tmpCheck1.toFloat();
Value2 = tmpCheck2.toFloat();
}
} else {
return false;
}
return compareValues(compare, Value1, Value2);
}

I recently split the functions of this code and it should not have changed functionality (apart from having the isFloat check now accepting leading spaces).
So I was wondering, why still perform the compareValues check when Value1 or Value2 is not a valid numerical value?

@TD-er
Copy link
Member

TD-er commented Oct 15, 2019

I think the function should be changed.

  • timeStringToSeconds should return a bool if it is a valid timeString (and then convert it to the float value)
  • If either one of them is a valid time string (N.B. 12 is also valid as in 12 hours) and at least one is not a numerical, then consider both sides a time notation.
  • If not a time notation, then try converting them to a numerical.
  • If neither is a numerical return false.

@uzi18
Copy link
Contributor

uzi18 commented Oct 15, 2019

maybe something is wrong with isFloat()/isNumerical() function

@TD-er
Copy link
Member

TD-er commented Oct 15, 2019

Very well possible, only I cannot see what.
I have looked at them for a while but cannot see what's wrong.

What does bother me is that we now have several functions to check numericals.
For example the validIntFromString function may turn 12:34 into the valid integer value 12.
But isNumerical will return false on 12:34

@TD-er
Copy link
Member

TD-er commented Oct 15, 2019

OK, took me a while, but I can now say that the change of &para into something else is a browser change in the logviewer
When printing the log to serial it does show perfectly fine.

254535 : Rule line  : LogEntry,"/json.htm?type=command&param=udevice&idx=663&nvalue=[rssi#rssi]"
254575 : Rule action: LogEntry,"/json.htm?type=command&param=udevice&idx=663&nvalue=-59"
254576 : ACT  : LogEntry,"/json.htm?type=command&param=udevice&idx=663&nvalue=-59"

(I added the "Rule line" and "Rule action" lines here for debugging to see what may change the string)

@uzi18
Copy link
Contributor

uzi18 commented Oct 15, 2019

ok so not a bug

@TD-er
Copy link
Member

TD-er commented Oct 15, 2019

Well, I'm still looking into the other parts of the code matching and parsing numericals.

@berbergh
Copy link
Author

Interesting finding, but if the sentence remains intact, why does it noet work anymore. The same sentence did work some odd weeks ago.

@TD-er
Copy link
Member

TD-er commented Oct 15, 2019

I did find some things with the compare functions which could in some edge cases yield incorrect results.
So I'm now working on a fix for it and will have a test build ready soon (past 20h, due to dinner and putting daughter to bed)
Would be nice though if it could still be included in the next build.

@TD-er
Copy link
Member

TD-er commented Oct 15, 2019

Can you test this quick build:
normal_core_260_sdk222_alpha_ESP8266_4M1M_testrules_issue2660.bin

@berbergh
Copy link
Author

Fair enough. Daughters are very important.
But for your information, I tried

Build:⋄ | 20103 - Mega
System Libraries:⋄ | ESP82xx Core 2_4_2, NONOS SDK 2.2.1(cfd48f3), LWIP: 2.0.3 PUYA support
Git Build:⋄ | mega-20190830
Plugins:⋄ | 80 [Normal] [Testing]
Build Md5: | b0a9a3f7ffce6b247d5c5fd19923f99a

And it works correctly in Domoticz, but the string is still weird:
sendtohttp myurl,8080,/json.htm?type=command¶m=udevice&idx=663&nvalue=380&svalue=0

TD-er added a commit to TD-er/ESPEasy that referenced this issue Oct 15, 2019
Detecting time compare and value compare could give false matches in some cases.
@berbergh
Copy link
Author

Your code won't load. The Wemos quits working and has to be power cycled

@TD-er
Copy link
Member

TD-er commented Oct 15, 2019

Hmm, that's strange.
I did check it only with a custom build, and made a 'normal build' just for you to test.
This is the one I did flash to my unit (not all plugins included)
Custom 4M1M issue 2660.bin

@berbergh
Copy link
Author

Something else is wrong. I can't even load the latest previous release anymore

@berbergh
Copy link
Author

I managed to load your code. Unfortunately it does not help.

@TD-er
Copy link
Member

TD-er commented Oct 15, 2019

OK, thanks for testing.
Can you get some logs on what it tries to send and also from a working node to see what the URL used to be?
What plugin is collecting the data?
Is there any task with the same name?

Can you give the output of the /json URL?

@uzi18
Copy link
Contributor

uzi18 commented Oct 15, 2019

first you should check this url in your browser:
http://myurl:8080/json.htm?type=command&param=switchlight&idx=531&switchcmd=Toggle

@berbergh
Copy link
Author

myurl actually is my Domoticz host on a rpi.
Your string returns this:
{
"status" : "OK",
"title" : "SwitchLight"
}

@uzi18
Copy link
Contributor

uzi18 commented Oct 15, 2019

so it should work, you can try to use ip address instead myurl in rules

@berbergh
Copy link
Author

@TD-er The device collecting the data is a MHZ-19 CO2 sensor. It is only available in test mode.
The data I see in the log is:
MHZ19: PPM value: 450 Temp/S/U values: 25/0/0.00
12871493: EVENT: CO2#PPM=450.00
12871518: ACT : sendtohttp myurl,8080,/json.htm?type=command¶m=udevice&idx=663&nvalue=450&svalue=0

And the rules are:
on CO2#PPM do
sendtohttp myurl,8080,/json.htm?type=command&param=udevice&idx=663&nvalue=[CO2#PPM]&svalue=0
LogEntry,/json.htm?type=command&param=udevice&idx=663&nvalue=[CO2#PPM]&svalue=0
endon

@TD-er
Copy link
Member

TD-er commented Oct 15, 2019

Can you also give the output of the /json url of ESPeasy?
Just to see if there's something fishy with the names of the names of the plugin or other plugins.
Thing is, I also added some caching to the name lookup of the tasks + values and I just want to make sure the error is not happening in there.

Does Domoticz also have some hints in the logs to see what's going on?

@berbergh
Copy link
Author

berbergh commented Oct 16, 2019

I have no clue how to get the output of espeasy. In the log of domoticz are no hints at all.

And last but not least, i am away from home for a week.

@TD-er
Copy link
Member

TD-er commented Oct 16, 2019

The json page is just in the browser.
Tools => Show JSON

Or you use the url http://<ip>/json
It is just plain text, so cut and paste.

TD-er added a commit to TD-er/ESPEasy that referenced this issue Oct 24, 2019
…#2660)

The changed rules parsing may have added some spaces in replaced variables.
Adding a trim() call to the parseString does remove any existing or added spaces in the parameter.
@TD-er
Copy link
Member

TD-er commented Oct 25, 2019

I made a test build which also includes a lot of other changes.
Can you please test it to see if sendtohttp now works like it should and also if I broke something else?

@berbergh
Copy link
Author

berbergh commented Oct 25, 2019 via email

@TD-er
Copy link
Member

TD-er commented Oct 25, 2019

Hmm P053 is included in the "testing" build and those failed to build in my test build due to size restrictions.
So I'm afraid those are not included in the build I linked.

@berbergh
Copy link
Author

@TD-er Your last test build works with respect to the json update.
I tested:

Build:⋄ 20104 - Mega
System Libraries:⋄ ESP82xx Core 2_5_2, NONOS SDK 2.2.1(cfd48f3), LWIP: 2.1.2 PUYA support
Git Build:⋄  
Plugins:⋄ 46 [Normal]
Build Md5: 727386f31985568f01f4d7798f8c07
Md5 check: passed.
Build Time:⋄ Oct 25 2019 03:41:41
Binary Filename:⋄ ESP_Easy_mega-20191016-11-PR_2667_normal_ESP8266_4M1M.bin

Would like to have Nextion and the PMSx003 included :-)

@TD-er
Copy link
Member

TD-er commented Oct 26, 2019

Can you try custom_ESP8266_4M1M_issue2660_Nextion.bin this one?

@berbergh
Copy link
Author

Works fine on DUST and Nextion.

@TD-er
Copy link
Member

TD-er commented Oct 26, 2019

Thanks for testing.
Good to hear I did at least fix it for this issue.
Let's hope it is now also fixed for the other related issues.

TD-er added a commit that referenced this issue Oct 27, 2019
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 a pull request may close this issue.

3 participants