search

letscontrolit / ESPEasy

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

Global Sync not working when no controller is set active #893

Closed TD-er closed 6 years ago

TD-er commented 6 years ago

As reported here on the forum.

The check for "should something be done" must not only check for enabled controllers, but also for checked global sync.

uzi18 commented 6 years ago

Someone could want only sync via udp. So no controller needed.

18.02.2018 3:40 PM "Gijs Noorlander" notifications@github.com napisał(a):

As reported here on the forum https://www.letscontrolit.com/forum/viewtopic.php?p=24795#p24795.

The check for "should something be done" must not only check for enabled controllers, but also for checked global sync.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/letscontrolit/ESPEasy/issues/893, or mute the thread https://github.com/notifications/unsubscribe-auth/AAHOUyfADL3BlxYFnBPhmcp4hiVYI0e5ks5tWDZNgaJpZM4SJtiC .

uzi18 commented 6 years ago

@TD-er In #896 I don't understand intention of code here: PLUGIN_READ -> succes = false we send data by udp? for me not succes -> we have no data to send/calculate.

TD-er commented 6 years ago

The problem is with the LCD plugin. It was an attempt to get global sync working when no controllers are enabled. But I guess fixing it like that is digging deeper in the mess. Global Sync (ESPeasy proprietary protocol) should be treated like any other controller. But that's quite some change in code. At least it will make Global Sync less buggy.

uzi18 commented 6 years ago

Lets make plugin for global sync...

26.02.2018 1:15 PM "Gijs Noorlander" notifications@github.com napisał(a):

The problem is with the LCD plugin. It was an attempt to get global sync working when no controllers are enabled. But I guess fixing it like that is digging deeper in the mess. Global Sync (ESPeasy proprietary protocol) should be treated like any other controller. But that's quite some change in code. At least it will make Global Sync less buggy.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/letscontrolit/ESPEasy/issues/893#issuecomment-368481822, or mute the thread https://github.com/notifications/unsubscribe-auth/AAHOU22oXEHSrL2Rp7pja1WBy4EVNwYVks5tYqBwgaJpZM4SJtiC .

TD-er commented 6 years ago

not a plugin, a controller.

uzi18 commented 6 years ago

I mean Controller Plugin :)

psy0rz commented 6 years ago

yeah that would be awesome!

Also the current global sync network code uses fixed buffers and is very susceptible to buffer overflows. e.g. if a random packet arrives that is not a globalsync packet it will crash the ESP.

Maybe you could make that more robust as well. (e.g. do some more checking and replace strcpy thats used there with strlcpy)

ghost commented 6 years ago

If you consider it a mess right now, I'll start to work on this one. I think it needs some new calls to the controller plugins because global sync is more than just a controller.

TD-er commented 6 years ago

What functions should the global sync have?

The second part is just like a controller and that controller could do some extra housekeeping task of knowing its listeners.

Am I missing some other features of the Global Sync?

ghost commented 6 years ago

It automatically creates "virtual" sensors on a receiving device and it needs to push config data for that. But it should not be a real issue. I've already started on C013 as "ESPEasy P2P networking" controller. Advantage is that one can easily remove this plugin and get rid of lots of unwanted code if it's not needed. I guess only a few users have this running.

TD-er commented 6 years ago

I think you might underestimate how much it is being used. (or how useful it can be)

Maybe also an idea to show a list somewhere to see what virtual sensors are available? And how to act when such a sensor is (temporary) not available?

ghost commented 6 years ago

All global sync related stuff is now moved out to a dedicated controller plugin.

The basic UDP packet handling is still within the framework but Global sync messages are transferred and processed within the controller plugin.

I could have moved all to the controller plugin but we would have some issues with:

Maybe we should leave it like this for now?

Grovkillen commented 6 years ago

This is a great improvement @mvdbro . Super! And I think you are right with leaving it "as is" for now, to many rely on the way it is working right now... leave it be for a year or so and many will have moved over to the new way of doing UDP.

ghost commented 6 years ago

But we will get new issues posted as people are unaware that global sync stops working after they upgrade their unit's. They have to add the new controller and also tick the "Send to controller" box on sensors that used to have global sync active.

TD-er commented 6 years ago

I think we should also merge this to v2.0, right?

ghost commented 6 years ago

I've done all this work because this issue is tagged with the V2.0 milestone. So i guess yes. But this is just because you guys wanted it. It's now a plugin, but I assume the original issue still exists.

ghost commented 6 years ago

Ask the person on the forum to use the latest version and check the logs like this: Sending unit 150500 : C013 : Send UDP message to 1 150511 : C013 : Send UDP message to 3 150521 : C013 : Send UDP message to 4 150532 : C013 : Send UDP message to 5 150543 : C013 : Send UDP message to 6 150553 : C013 : Send UDP message to 7 150624 : C013 : Send UDP message to 14 150634 : C013 : Send UDP message to 15 150665 : C013 : Send UDP message to 18 150786 : C013 : Send UDP message to 30

Receiving unit: 11678439 : UDP : 60:01:94:0F:A2:3F,192.168.0.102,2 11688458 : C013 : msg 5 2 18 10 10 11689498 : UDP : 5C:CF:7F:0F:50:BC,192.168.0.106,6 11690156 : C013 : msg 5 2 18 11 11

uzi18 commented 6 years ago

It would be nice to have information on task list "data from global sync" and from where they come.

TD-er commented 6 years ago

Is this one fixed, now global sync is moved to be a separate controller?

blb4github commented 6 years ago

Hi @mvdbro, I'm struggling with this new global sync controller setup. I see the C013 messages on both sending and receiving unit, but I don't get the task copied on the receiving unit. What do I have to do to get this working on the receiving unit?

ghost commented 6 years ago

It was confirmed on the forum that's it working.