Issue with P079 since mega-20201227

TungstenE2 commented 3 years ago

Summarize of the problem/feature request

Hi all,

I recently discovered an issue with my chicken door operated using the P079 Wemos motor shield plugin.

https://espeasy.readthedocs.io/en/latest/Plugin/P079.html#p079-page

Installation and rules were running stable for a long time. I recently updated to ESP_Easy_mega_20210114_normal_ESP8266_4M1M.bin and I randomly discovered issues with my sunrise rule to open the door.

I realized that there where cleanup activities to the plugin with Release mega-20201227.

May be these cleanups are causing the issue?

@TD-er: could you check if the clean up done with mega-20201227 caused an issue?

I just rolled back to ESP_Easy_mega_20201130_normal_ESP8266_4M1M.bin and this seemed to fix the issue.

thx

Expected behavior

see above

Actual behavior

see above

Steps to reproduce

see above

System configuration

Hardware: Wemos D1 mini with wemos motor shield

ESP Easy version: mega-20201227

ESP Easy settings/screenshots: grafik

Rules or log data

TD-er commented 3 years ago

What kind of issues do you experience?

TungstenE2 commented 3 years ago

Oh sorry, I missed this detail.

The open door command in the rule is not correct executed. The door remains closed. On the other hand I am able to open the door using my little android app or by manual switch pushing which also executes a rule. I did not change any rule for several months. But after I installed the mega-20201227 with the code cleanup I discoverd issues just with the door opening rule in the morning. Everthing else is still working as expected. Just the chickens are locked in... ;-)

grafik

TD-er commented 3 years ago

Ohoh, I'm seeing an angry chick. Let's add another tag to this issue :)

TungstenE2 commented 3 years ago

Let me know if you need additional info.

Yesterday I rolled back to ESP_Easy_mega_20201130_normal_ESP8266_4M1M.bin and this morning everything was working as expected.

TungstenE2 commented 3 years ago

Additional info, the rule for DoorOpen is the one having an issue.

With ESP_Easy_mega_20201130_normal_ESP8266_4M1M.bin this is working fine again the last 2 days.


//Events

On DoorOpen Do
If [Reed1-open#Switch]!=1
    WemosMotorShieldCMD 0 Backward 50
    TaskValueSet 7,3,1
endif
EndOn

On DoorClose Do
    If [Reed2-close#Switch]!=1
    WemosMotorShieldCMD 0 Forward 50
    TaskValueSet 7,3,1
endif
EndOn

On DoorStop Do
    WemosMotorShieldCMD 0 Stop
    TaskValueSet 7,3,0
EndOn

TD-er commented 3 years ago

I do notice you have a - sign in the task name. Not sure if that will work fine.

TungstenE2 commented 3 years ago

Working fine for about 2 years now.

TD-er commented 3 years ago

Working fine for about 2 years now.

Well since last build(s) it isn't and as there has been quite a lot of updates in the rules parsing and not so much in the motorshield code, I wonder if it is the rules I should look into or in the motorshield (or something inbetween)

TungstenE2 commented 3 years ago

so should I change '-' to '_' in the Task/Device Name?

grafik

TungstenE2 commented 3 years ago

still using the old Rules Engine

grafik

TD-er commented 3 years ago

That "old" engine label shoulf be renamed to something else, as the "new" engine is not going to be used in the future as it is not really usable and there are other ways to speed up rules parsing. So "old" is good :)

You can try to use an underscore, or use no character between them. Task names and task variable names are not case sensitive, so you can use camel case if you would like to make it clear.

TungstenE2 commented 3 years ago

Ok, need to find sometime to test. The only problem ist that the issue only occurs for the morning rule to open the door by sunrise. And it seems like it is only randomly not working. First I discovered that Task 7 was missing dummies 2 and 3 after an update. So I added them again and the next day it was working again. But failed again 1 or 2 days later again. I than rolled back to older version and it was working again directly.

TungstenE2 commented 3 years ago

@TD-er did some testing this morning.

removed '-' from task/device names
updated all rules accordingly
updated FHEM devices and doifs accordinly as well
verfied every thing still working (incl Telegram messages upon close/open)
updated from ESP_Easy_mega_20201130_normal_ESP8266_4M1M.bin to ESP_Easy_mega_20210223_normal_ESP8266_4M1M.bin
this is when the trouble started...
got browser message: Low memory. Cannot display webpage :-(
Load 100.00% (LC=1), usually this was much lower
Tried to controll the door using my android app.
Door was working but with big delay
Door got stuck, as the upper reed sensor did not stop the door due to the delay and system load
had to crawl with a screw driver into the chicken house and fix the stuck door
roll back after 2 hrs to ESP_Easy_mega_20201130_normal_ESP8266_4M1M.bin
system load now 63.46% (LC=1372)
at least I left the house during corona for a short time, even it was raining... ;-)

please advice

TungstenE2 commented 3 years ago

did another test....

update from ESP_Easy_mega_20201130_normal_ESP8266_4M1M.bin to ESP_Easy_mega_20210114_normal_ESP8266_4M1M.bin

Seems to work and system load is about 65%.

Just need to see if the sunrise rule is working again, as reported in the beginning of this issue.

TD-er commented 3 years ago

Maybe also good to test a build first before potentially bothering the chickens? ;) Do you use "hidden SSID" access points?

TungstenE2 commented 3 years ago

no hidden SSID used, but my Unifi APs are using a hidden SSID, not sure why exactly.

As I have no test setup with spare motorshield etc the chicken are my testers. And trust me, they find every bug... ;-)

TD-er commented 3 years ago

They do sound rather picky ;)

TungstenE2 commented 3 years ago

update: I can confirm that ESP_Easy_mega_20210114_normal_ESP8266_4M1M.bin is working again as expected after removing '-' from task/device names and rules.

But as staded yesterday ESP_Easy_mega_20201130_normal_ESP8266_4M1M.bin seems to have an issue with load and low memory.

TD-er commented 3 years ago

I'll keep an eye out for other (probably WiFi related) issues with the latest build. Until now there have been rather surprisingly few reported issues with the latest build so far.

TungstenE2 commented 3 years ago

Hi @TD-er the issue is back. On Sat and Sun I have a differnt configuration during weekdays. On weekdays the sunrise time triggers rule to open the door. On Weekends I set it to 8:30am. Yesterday and today I could verify that the rule starts the motor for 1 sec and than stops again. I checked the Log and it seems that the close reed triggers a DoorStop Event. But I do not understand this. I did not change these rules and it was working fine for a long time. The issue only occured after I started updating. During the week with %sunrise% this is working fine. Also manual opening and closing is working fine, and the same rules for DoorStop are applied.

Any idea?

grafik

TD-er commented 3 years ago

Looks like it may have been a 'bounce' of the reed switch? Could it be that the switch may have been triggered by some mechanical vibration?

TungstenE2 commented 3 years ago

I had the same idea, but I have some doubts. Why would this only happen on weekends? And today I was watching the door and did not see any unusual movement. I will further observe it without changing the settings. I would bet that Mo-Fri it will work as expected. Will let you know tomorrow morning... ;-)

TD-er commented 3 years ago

Can you check with the rules using 08:30:00 instead of 08:30 ?

TungstenE2 commented 3 years ago

sure I can test, but I doubt this beeing the reason, as the event DoorOpen is triggered correct, Some how the event DoorStop is triggered to early on Sat/Sun.

tonhuisman commented 3 years ago

You haven't set a de-bounce value on your reed switch (according to the screenshot above). For any switch I'd use a de-bounce value, especially for an end-switch that stays in a specific state when that position is reached, as there is no expected reason to go to that state again, unless the door is moved in the other direction first. And we can't see your DoorOpen and DoorStop code, does that reset the timers in some way?

And having several event handlers for %sunrise% and probably also %sunset%, you could put that into 1 event handler (to lower system load a little) that then checks %sysweekday%, f.e. like this:

...
on Clock#Time=All,%sunrise% do
  if %sysweekday%=1 or %sysweekday%=7
    LogEntry,"Weekend! %eventvalue1%"
    // Not working day
  else
    LogEntry,"Workday mon-fri: %eventvalue1%"
    TimerSet,1,60 // Working day
  endif
endon

The events for Saturday and Sunday 08:30 you should probably keep.

TungstenE2 commented 3 years ago

@tonhuisman thx for your feedback and input.

Here is my DoorOpen, DoorClose and DoorStop code. grafik

What exactly do you mean by 'de-bounce' value. Can you give an example of your code used?

My setup was workingt fine and stable for a very long time, issues started with mega-20201227 I assume, but hard to say what caused the issues.

tonhuisman commented 3 years ago

What exactly do you mean by 'de-bounce' value. Can you give an example of your code used?

De-bouncing is stabilizing the action of a switch, the state of the switch has to be in the 'on' state for the de-bounce time before it is accepted and acknowledged as being changed. This filters out spurious changes and signal-spikes that can be picked up by the wiring from external sources, as long wires act as an antenna. A switch accepts up to 250 msec as the de-bounce value. For a switch that needs to give a steady but quick response, I'd set it to 50, if stability is more important, and response doesn't really matter much, a value between 100 and 200 msec seems feasible. As reed switches aren't the most firm construction, but your setup requires a prompt response, I'd set it to 50. If it responds too slow, a value of 20 might be just right.

tonhuisman commented 3 years ago

issues started with mega-20201227 I assume, but hard to say what caused the issues.

I think that in that release, many code improvements are included, making ESPEasy more responsive, IMHO, making setups, depending on previous 'delays' or 'slugishness', somewhat less stable, even though the overall stability and speed have improved. Adding some stabilizing settings, like the de-bounce value for switches (I use that on all switches used with my ESP's, to avoid soldering capacitors on input pins), will improve your projects.

TungstenE2 commented 3 years ago

ok, now I got you. You are referring to the switch advanced event management value 'De-Bounce'.

To be honest I never realized what this is for. I now set it to 50, as suggested. grafik

Thanks a lot for this advice!

giig1967g commented 3 years ago

Hi all, I am the one who coded the de-bounce and safe button. First of all, let's say that switches states are checked every 100ms (10 times per second). So any value below 100ms is useless.

De-bounce: When you activate a switch, the de-bounce "ignores" every state change within the set interval. So if switch oscillate between 0 and 1 within the set interval, these state changes do not trigger an event. A correct de- bounce value is 100-200ms, as 50ms it's too low as the single cycle is 100ms.

"safe button": The safe button waits two 100ms cycles before firing the trigger, in other words, the switch must remain in the state position for at least 100ms. It avoids false positive. It's drawback is that you have to keep the button pressed for at least 100ms. A very quick press and release might not fire the event.

If you need more explanations please ask.

TungstenE2 commented 3 years ago

Hi @giig1967g, thx for explanations.

So can I assume this to be the right configuration for my setup?

grafik

TungstenE2 commented 3 years ago

btw: shouldn't 'Use Safe Button' be named 'Safe Button', as the checkbox means 'use'?

giig1967g commented 3 years ago

@TungstenE2 Yes, that should be fine.

Regarding the label, you are right! Will change it.

TungstenE2 commented 3 years ago

@giig1967g thx.

Regarding the label I would even suggest to name it different. The device is a switch. So may be 'Safe Switch', 'Safe State', 'Safe Switch State' or similar would be more intuitive and user friendly.

tonhuisman commented 3 years ago

switches states are checked every 100ms (10 times per second). So any value below 100ms is useless.

OK, but looking at the code I'd guestimate that any value > 10 will result in a 100 msec de-bounce. So that's still good enough.

Regarding the label, you are right! Will change it.

Will that include the (partially missing ReadTheDocs (RTD)) documentation? (As the Wiki documentation is really outdated)

TD-er commented 3 years ago

Actually the functionality is "Delay Accept".

giig1967g commented 3 years ago

OK, but looking at the code I'd guestimate that any value > 10 will result in a 100 msec de-bounce. So that's still good enough.

ok, I have coded it few years ago... I might have forgotten something :(

Will that include the (partially missing ReadTheDocs (RTD)) documentation? (As the Wiki documentation is really outdated)

Will do my best to update the documentation.

Actually the functionality is "Delay Accept".

You want me to change the label to "Delay Accept"?

TungstenE2 commented 3 years ago

I am getting confused.... How can value 10 result in 100 msec if the vlaue is labled with (mesc)?

tonhuisman commented 3 years ago

How can value 10 result in 100 msec if the vlaue is labled with (mesc)?

The check is done every loop (100 msec). The first de-bounce check is done in the first loop, but the time passed since that first press will (most likely) be less than 10 msec, so it will not be accepted. In the next loop, 100 msec later, the state should be accepted, assuming the state hasn't changed, effectively giving ~100 msec de-bounce delay.

TD-er commented 3 years ago

Maybe we should then set a minimum of 100 in the input field?

TD-er commented 3 years ago

You want me to change the label to "Delay Accept"?

Not the best name I guess, so if you have a better suggestion? The label should describe what it does from the user's viewpoint. "Safe button" what it is called now, is more a generic intention and not describing what it does. But since the users of ESPEasy are a bit more tech minded, I think it should describe the actual effect.

So maybe it is more like "Require extra 100 msec state duration", but that's too long for a label. (could be a note)

So we're now at the hardest part of programming... thinking of a good descriptive name for a parameter.

giig1967g commented 3 years ago

From the user point of view we could describe the effect: we could call it "Avoid false positive" or "Reduce risk of false positive"

TD-er commented 3 years ago

That's also an intended "side effect" and not describing what it does.

What it does is, it will make sure the state is stable for another 100 msec. Essentially extending the "debounce time", right?

"Avoid false positive" is some promise you can't keep up.

tonhuisman commented 3 years ago

If stated like that, even '(Use) Safe Button' is such a promise...

Possibly this setting should be removed, and a clear note added to the De-bounce setting. (And also allow for somewhat longer de-bounce time than 250 msec, possibly up to 500 msec)

TD-er commented 3 years ago

The debounce period may allow a few triggers inbetween. The now called "Safe Button" mode requires the state to remain stable for 100 msec. So it is not really the same.

It is more like "Require 100 msec stable state" :)

TungstenE2 commented 3 years ago

shouldn't the feature be moved below the 'De-Bounce' value, as they are related?

grafik

TD-er commented 3 years ago

Yep.

TungstenE2 commented 3 years ago

Hi all,

so what are the next steps and who needs to take a decission?

I see several options and different views.

De-bounce:

set minimum value

Safe-Button:

renaming to 'De-bounce plus' with detailed description in documentation what 'plus' means
or may be even removing
moving the feature below the de-bounce setting

Documentation:

Updating the documentation with examples on how to use the settings.

giig1967g commented 3 years ago

@TungstenE2 first of all, does the de-bounce setting solved your problem?

TungstenE2 commented 3 years ago

@giig1967g in order to answer this I would suggest that I go on with my testing for the next week. I have 2 reed switches. The issue only occured for the lower reed indication the door is closed and for the DoorOpen event in the morning. Not 100% sure if it only happend on weekends.

Yesterday I changed the settings as suggested and this morning it was working fine. But last week it was also working fine Mo-Fr. So lets wait for the weekend.

But the de-bounce settings sound like a solution to such a situation with reed switches not beeing 100% reliable.

But independed of my issue the topics I listed should be be solved in general, right?

letscontrolit / ESPEasy