letscontrolit / ESPEasy

Easy MultiSensor device based on ESP8266/ESP32
http://www.espeasy.com
Other
3.24k stars 2.2k forks source link

JSON service stopped working #2697

Closed tiomny closed 4 years ago

tiomny commented 4 years ago

I flashed the latest release to my ESP8266 and realized that json page stopped working. When I open http:///json I don't receive any data from the ESP.

ghtester commented 4 years ago

The same on my custom build from latest sources (mega-20191028), I see only blank page instead of JSON.

TD-er commented 4 years ago

Yep, confirmed. Will have a look at what's happening here.

TD-er commented 4 years ago

Found the issue. The default value for "taskindex" was not updated, when I did change the internal taskindex into a uniform type all over the code.

The main problem was that all over the source code some kind of integer type was used for TaskIndex:

Now it is all using the same typedef value, of uint8_t, but it also means the value can no longer be negative and when you use a value of -1 as a default value, strange things may happen.

The fix also fixes the automatic update of the values on the Devices tab.

micropet commented 4 years ago

I do not know if my problem has anything to do with it:

I have a device with a MH-Z19 Co2 sensor. It sends the Co2 value to another device with a Neopixel Stripe with the following command:

Publish ESP-206/cmd, NeoPixelAll, 0,3,0,0

This does not work anymore in the current version compiled with Vagrant.

TD-er commented 4 years ago

@micropet It is not related. Can you try using quotes on the last argument like this:

Publish ESP-206/cmd,"NeoPixelAll,0,3,0,0"

If that's not working, please open a new issue for it.

micropet commented 4 years ago

Thanks, i do it.

micropet commented 4 years ago

quotes do not work, i make an issue for it, thanks.

TD-er commented 4 years ago

quotes do not work, i make an issue for it, thanks.

@micropet I found the (already reported) issue: https://github.com/letscontrolit/ESPEasy/issues/2662 And I have a fix for it. Test build will be available in less than an hour.

clumsy-stefan commented 4 years ago

Now it is all using the same typedef value, of uint8_t, but it also means the value can no longer be negative and when you use a value of -1 as a default value, strange things may happen.

Why did you opt for uint8_t and not byte? As we're anyway way below the 255 possible values of byte, wouldn't that save some amount of mem (if I^m not wrong, uint_t uses 2 bytes)

sanyafifa commented 4 years ago

My node-red stopped polling the device with the latest firmware

TD-er commented 4 years ago

@clumsy-stefan I was mistaken, it was typedef'ed to byte.

Can you try this test build: ESPEasy_mega-20191028-21-PR_2698.zip Please let me know if it does work and it would be great if you could do some more thorough testing so I can merge it for tomorrow's build. It does fix JSON and MQTT publish command

micropet commented 4 years ago

The testbuild does not work. The payload is not complete.

It would have to be sent NeoPixelAll, 250,170,90,90, but only NeoPixelAll will be sent.

The old (Aug 2019) versions work.

TD-er commented 4 years ago

Can you give the entire line what it does send? Have you also tried with the quotes in the test build? Like this:

Publish ESP-206/cmd,"NeoPixelAll,0,3,0,0"
TD-er commented 4 years ago

Just to give an explanation about why this is changed. The Publish command now uses the argument parser which does parse the line for arguments. Only a comma is an argument separator. Previously the Publish command just used the 3rd argument until the end of the line as "value". But that was also giving issues when people tried using quotes or a combination of quotes and parenthesis.

micropet commented 4 years ago

I have the following in my rules:

on gruen do if %systime% > 08:00:00 and %systime% < 20:00:00 Publish ESP-206/cmd,NeoPixelAll,0,250,0,0 //gruen tagsueber else Publish ESP-206/cmd,NeoPixelAll,0,3,0,0 //gruen abends endif endon

The quotes make no difference.

The payload is just NeoPixelAll, the rest is swallowed.

TD-er commented 4 years ago

@micropet Are you sure you have the latest test build flashed on that unit? As a matter of fact, I have the same line as you have tried on my test node and it does send the data just fine, as long as I use the quotes like this:

on gruen do
if %systime% > 08:00:00 and %systime% < 20:00:00
Publish ESP-206/cmd,"NeoPixelAll,0,250,0,0" //gruen tagsueber
else
Publish ESP-206/cmd,"NeoPixelAll,0,3,0,0" //gruen abends
endif
endon
TD-er commented 4 years ago

This publish line:

publish /topic/test,"NeoPixelAll,0,3,0,1" //gruen abends

And in my MQTTBox application: image

micropet commented 4 years ago

Oh shit, we just have over 1500 PPM because the windows are closed. And exactly with this line I forgot the quotes.

Everything is alright now. Thank you very much.

micropet commented 4 years ago

Something is not right yet.

We now have 516 PPM and it should be sent "green". But it is sent "violet" with "NeopixelAll, 250,0,40,0".

The version of 23.8 also sends "violet" when I use the quotes.

If I leave the quotation marks, the old version sends "green" properly.

This is the condition, as it has been working for many months.

So summarized: The quotes disturb.

micropet commented 4 years ago

That my Rules:

//----- on MH-Z19#PPM do if [MH-Z19#PPM]<700 event gruen endif if [MH-Z19#PPM]>=700 and [MH-Z19#PPM]<=1100 event gelb endif if [MH-Z19#PPM]>1100 and [MH-Z19#PPM]<1400 event violet endif if [MH-Z19#PPM]>=1400 event rot endif endon

on gruen do if %systime% > 08:00:00 and %systime% < 20:00:00 Publish ESP-206/cmd,"NeoPixelAll,0,250,0,0" //gruen tagsueber else Publish ESP-206/cmd,"NeoPixelAll,0,3,0,0" //gruen abends endif endon

on gelb do if %systime% > 08:00:00 and %systime% < 20:00:00 Publish ESP-206/cmd,"NeoPixelAll,250,170,0,0" //gelb tagsueber else Publish ESP-206/cmd,"NeopixelAll,4,2,0,0" endif endon

on violet do if %systime% > 08:00:00 and %systime% < 20:00:00
Publish ESP-206/cmd,"NeopixelAll,250,0,40,0" //violet tagsueber else Publish ESP-206/cmd,"NeopixelAll,4,0,1,0" endif endon

on rot do if %systime% > 08:00:00 and %systime% < 20:00:00
Publish ESP-206/cmd,"NeopixelAll,250,0,0,0" //rot tagsueber else Publish ESP-206/cmd,"NeopixelAll,4,0,0,0" endif endon //-----

TD-er commented 4 years ago

Did it show the yellow color inbetween? Or was there not any other update by the rules?

sanyafifa commented 4 years ago

@clumsy-stefan I was mistaken, it was typedef'ed to byte.

Can you try this test build: ESPEasy_mega-20191028-21-PR_2698.zip Please let me know if it does work and it would be great if you could do some more thorough testing so I can merge it for tomorrow's build. It does fix JSON and MQTT publish command

Oh thanks! JSON is back in work!

micropet commented 4 years ago

no yellow.

I have now put back the Aug 2019 version without quotation marks, as long as there is no solution.

uzi18 commented 4 years ago

Found the issue. The default value for "taskindex" was not updated, when I did change the internal taskindex into a uniform type all over the code.

The main problem was that all over the source code some kind of integer type was used for TaskIndex:

* byte

* uint8_t

* unsigned int

* int

* size_t

Now it is all using the same typedef value, of uint8_t, but it also means the value can no longer be negative and when you use a value of -1 as a default value, strange things may happen.

The fix also fixes the automatic update of the values on the Devices tab. @TD-er why not simple typedef int8_t taskIndex_t; it is just 0-11 or 0-23 + invalid value

TD-er commented 4 years ago

@TD-er why not simple typedef int8_t taskIndex_t; it is just 0-11 or 0-23 + invalid value

Most of the original implementation used byte and I did not want to change it where it is used in settings structs which are stored. Also I changed the PluginID type (also a typedef) and DeviceIndex. Those have to use an unsigned value, since for the plugins we already pass the 127 (playground plugins) and the DeviceIndex is also something that may pass the 127 some day. So to keep the logic for what's considered an invalid/valid value similar I chose to have them all unsigned.

The logic for PluginID is already a bit strange, since it was once decided that 0 would be the invalid one and we don't know how many there will be some day.

The code uses the DeviceIndex value to switch between PluginID and internal data structures. The code of a few weeks ago had to loop through all Devices (length MAX_DEVICES) to find the matching PluginID. I also just added a lookup map to make it very fast to switch from PluginID to DeviceIndex for example. All instances of these values now use the newly created typedef'ed types and wherever I could find it, they also use their isValid function to check. This alone gave a number of code paths where there never has been a proper check for these and a vector would directly be addressed without bound checking which can lead to crashes.

So it is now a lot less resource hungry and a lot more checks in the code. But I had to change a lot of places where taskIndex was not considered a byte but an int for example and if it were then set to -1 as invalid value, I had to correct it. It could receive the value from somewhere where it was an unsigned value.

So that's why I chose an unsigned type for it :)

TD-er commented 4 years ago

@micropet I have no idea why your rules do seem to work like they do apparently. I have just started a new test build, which I will copy/link tomorrow morning (if it builds fine) and then it would be nice if that one could be tested. I have added some more debugging on the rules parsing with regards to if ... and and or statements

TD-er commented 4 years ago

I have a latest test build. ESPEasy_mega-20191028-31-PR_2698.zip

micropet commented 4 years ago

It still does not work. In the Rules I have, as you said, the quotation marks. At 540 PPM the device sends "violet", ie "NeopixelAll, 250,0,40,0" It should be "green" "NeoPixelAll, 0,250,0,0".

It sends short "green", then comes immediately "violet"

Here is an excerpt from the log:

 Command: event 245916: EVENT: violet 245963: ACT: Publish ESP-206 / cmd, 'NeopixelAll, 250,0,40,0' 245972: Command: publish 246060: EVENT: MH-Z19 # MHZ19Temp = 22.00 246129: EVENT: MH-Z19 # U = 0.00 272810: WD: Uptime 5 ConnectFailures 0 FreeMem 16344 WiFiStatus 3 275829: ACT: Publish ESP-206 / cmd, 'NeoPixelAll, 0,250,0,0' 275838: Command: publish 275885: ACT: event violet 275893: Command: event 275893: EVENT: violet 275940: ACT: Publish ESP-206 / cmd, 'NeopixelAll, 250,0,40,0' 275949: Command: publish 276013: EVENT: MH-Z19 # MHZ19Temp = 22.00 276078: EVENT: MH-Z19 # U = 0.00 278255: EVENT: Clock # Time = Sat, 10:18 302810: WD: Uptime 5 ConnectFailures 0 FreeMem 16328 WiFiStatus 3 305854: ACT: Publish ESP-206 / cmd, 'NeoPixelAll, 0,250,0,0' 305864: Command: publish 305908: ACT: event violet 305916: Command: event 305916: EVENT: violet 305965: ACT: Publish ESP-206 / cmd, 'NeopixelAll, 250,0,40,0' 305974: Command: publish 306036: EVENT: MH-Z19 # MHZ19Temp = 22.00 306101: EVENT: MH-Z19 # U = 0.00

micropet commented 4 years ago

What amazes me: Co2 is not displayed in the log. I only see the lines:

996034: EVENT: MH-Z19 # MHZ19Temp = 21.00 996128: EVENT: MH-Z19 # U = 0.00

No PPM

The Co2 value is displayed on the device page.

Unbenannt

tiomny commented 4 years ago

Why are you discussing that under this ticket?

TD-er commented 4 years ago

@micropet Can you open a new issue about this? It may be related to the CO2 plugin itself and for sure it is not related to the JSON output, as @tiomny also points out.

micropet commented 4 years ago

okay

TD-er commented 4 years ago

This one should be fixed now, so I will close it. Please let me know if it still is an issue.