domoticz / domoticz

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

Device sharing issue with Python Plugin #2642

Open g-chevalier opened 6 years ago

g-chevalier commented 6 years ago

Version: 4.9700 Platform: Raspberry Pi

I setup sharing between two Domoticz installations and it seems that devices originated and managed by a Python Plugin don’t seem to be shared.

So, I’m wondering if Device sharing is supported with Python Plugins…

I’m convinced I did everything OK in the master / slave setup as I immediately see the slave Domoticz internal Devices such as CPU temp, memory percentage shared on the master instance. But I never saw other devices that are under control of the Python Plugin, even after a long wait.

The Python plugin is a plugin I’m currently developing, and it works well on the Domoticz perspective.

To test, I also added a virtual sensor on slave that is updated by one of my sensors managed with the plugin and shared it. It works: I can see the shared device in the master instance, and the value is updated when the slave device is changed.

gizmocuz commented 6 years ago

It depends how internally a sensor/device is updated (i have currently no idea how this is done via python) If it is directly updating values in the database, then it is not going to work. If it is using the functions in DomoticzHardware.cpp/h, it should work Maybe one of the python integrator's can comment on this

gizmocuz commented 6 years ago

Could this be the same issue ? https://github.com/domoticz/domoticz/issues/2484

g-chevalier commented 6 years ago

Sure, the issue seems really the same, although this ticket is with mine plugin I just created.

For information, I don’t write directly to DB! I am going through the Python Plugins API “Devices[id].Update(…)”

gizmocuz commented 6 years ago

but what is “Devices[id].Update(…)” calling under the hood ?

g-chevalier commented 6 years ago

That’s a good point.

I was fooled into thinking that this function calls appropriate Domoticz internals, but if I understand well with my limited knowledge of the inner Domoticz, it seems it makes its own DB update more or less directly. Bellow is what I found.

It looks like that in the Python plugin API, it is the CDevice_update function in hardware\plugins\PythonObjects.cpp that is called. And exploring the code, it seems that it starts by calling CSQLHelper::UpdateValue and then checks for some interactions such as scene / group triggering (CheckSceneCode function) and MQTT + various push mechanisms (sOnDeviceReceived function), then some notifications (CheckAndHandleNotification function).

However, as you said at the beginning, it would be far better that “one of the python integrator's can comment on this”. I may totally miss-understand something.

emontnemery commented 6 years ago

@gizmocuz, you write

If it is using the functions in DomoticzHardware.cpp/h, it should work

Python is deriving the class DomoticzHardware as are all other hardware classes as I understand it. Can you be a bit more specific, which functions should be used?

gizmocuz commented 6 years ago

@emontnemery , all functions in DomoticzHardware.cpp like SendTempSensor

emontnemery commented 6 years ago

@gizmocuz OK, but what should be used for Plugin controlled switches for example? CDomoticzHardwareBase::SendSwitch is using the (to my understanding) old/deprecated LIGHTING2 switch type which only supports 16 levels for dimming. Should CDomoticzHardwareBase be extended with functions for GeneralSwitch?

gizmocuz commented 6 years ago

No, we have SendGeneralSwitch

emontnemery commented 6 years ago

OK, Now I see what you mean. The difference is that the plugin bypasses mainworker's message loop, where this call would have been made: m_sharedserver.SendToAll(pHardware->m_HwdID, DeviceRowIdx, (const char*)pRXCommand, pRXCommand[0] + 1, pClient2Ignore); Instead plugin framework simply calls m_sql.UpdateValue()

I think you mentioned before that all HW types should be modified to use CDomoticzHardwareBase helper functions?

gizmocuz commented 6 years ago

Yes that would be ideal ... But i was working (before i had a break), on another feature, and once that feature is ready/working, then it would be very easy to solve this as well.

I was implementing 'cereal', see hardwaretypes.h.

This needs to be done for all known structures.

The challenge is going to be to override a union for the RFXCom sdk (in hardwaretypes.h) (I have no idea/solution yet to override a union, else we need to make structs like 'LIGHTING1_Ex', 'LIGHTING2_Ex' etc)

Then i was thinking about making one new 'send' function, that checks the pType/pSubType, and based on that cast/fill the correct structure, and serialize to JSon (and back)

dnpwwo commented 6 years ago

Perhaps now is the time to move to an object hierarchy under CDomoticzHardwareBase and move away from long switch statements based on types and subtypes?

Polymorphism would replace that and allow real encapsulation so that there would be way less accidental side effects when devs are changing things. Objects would just need to know how to serialize themselves over and above the base class(es) serialization.

gizmocuz commented 6 years ago

@dnpwwo , please propose a better way with object. For now, everything is already in structures (kinda objects), that can be serialized with cereal

emontnemery commented 6 years ago

@gizmocuz Here's some discussion about using cereal with unions: https://groups.google.com/forum/#!topic/cerealcpp/8gpW-W9qN9I

dnpwwo commented 6 years ago

My question would be what are you trying to serialize?

If it's hardware then we should add a virtual Serialize functon to DomoticzHardwareBase that returns the string version of the base. If other hardware needs to do anything above that it can call the base function and then enhance the output. That way there is no switch statement on Type.

If you want to serialize devices that is a bigger problem because there is no DomoticzDeviceBase (which is a shame because I think that would be a great thing !), maybe this is the trigger to create one.

gizmocuz commented 6 years ago

@dnpwwo , it's for serializing structures (devices). All devices are already in structures (objects if you want to call 'm like that), and can be serialized to JSON for example, send over, and serialized back. Nothing complex

hansake commented 5 years ago

I have the same device sharing issue when using a python plugin for handling TP-Link Smartplug HS110, downloaded from [https://github.com/dahallgren/domoticz-tplink-smartplug/blob/master/plugin.py].

This plugin works well for non sharing when slightly modified. Change line 198 - 200 in plugin.py to:

Devices[2].Update(nValue=0, sValue=str(realtime_result['current_ma'] / 1000))
Devices[3].Update(nValue=0, sValue=str(realtime_result['voltage_mv'] / 1000))
Devices[4].Update(nValue=0, sValue=str(realtime_result['power_mw'] / 1000))
soulslider commented 5 years ago

Hello, any news on this subject ? The python plugin API is still not working for shared devices ?

gizmocuz commented 5 years ago

As you might have noticed in the recent commits, cereal support has just been merged in the beta again Next is the testing part, and when this works, we need to use it

soulslider commented 5 years ago

Thanks for your quick reply ! I hope the testing part to be postive

pipiche38 commented 5 years ago

Would we expect also a positive impact on Python plugin around all sensors and especially THB which currently doesn't benefits from the Forecast and Trend analysis that native plugin have N

pviolero commented 3 years ago

I have the same issue when I try to use the python zigate pluggin on a slave domoticz (master & slave with the release 2020.2

Do you have some news regarding potential correction of this issue ?

mcostantini70 commented 3 years ago

I have exactly the same issue with 2020.2 and zigate plugin. Is this going to be resolved?

gizmocuz commented 3 years ago

Did you try the beta ?

mcostantini70 commented 3 years ago

Did you try the beta ?

Is it addresses in the Beta?

pipiche38 commented 3 years ago

Is #2484 addressed in the recent beta ?

gizmocuz commented 8 months ago

Please check beta 15790 ... Both client/server need to run on this beta... quite some things have changed. You probably will get new devices, so make sure 'Accept new hardware/devices' is enabled in the settings