Safe adding device

[szczukot]

No description provided.

[szczukot]

Reference to #2146

[gizmocuz]

Thanks for the patch !

Would it not be better if all those hardware classes where using the 'general' way of adding/sending sensors in domoticz by using the functions

void SendSwitch(const int NodeID, const int ChildID, const int BatteryLevel, const bool bOn, const double Level, const std::string &defaultname, const int RssiLevel = 12);

void SendSwitchIfNotExists(const int NodeID, const int ChildID, const int BatteryLevel, const bool bOn, const double Level, const std::string &defaultname);

There is on hardware class that is changing the switch type, with this patch the new function is always returning the new ID which causes all calls to be slower... and 'UpdateValueInt' is used quite a lot..
Not sure if it would be better to just do the switchtype changing in this one place (or create a function for this too)

[szczukot]

SendSwitch etc works only for switch. New method is working for all types of device.

Method returns ID, because one (only eHouseTCP) of call using it :(
But I think it is not a big problem. This method is called practically only once for each device during its creation. Never again.

This method don't calls UpdateValueInt.

It is universal. Now nobody will call "INSERT INTO DEVICESTATUS..." without permision etc

The next good change is that you do not have to call "UPDATE DEVICE STATUS" for change switchtype etc. after insert.

And this method is better that UpdateValueInt or SendSwitch etc, because it only add device. Not calls Events, notifications etc. Because for new device, this 'things' not exists yet.

[gizmocuz]

UpdateValueInt is called by 'CSQLHelper::UpdateValue' which is called a lot
You could add the code only in the eHouseTCP class

But again, It would be good if all classes used the basic/standard way of adding/updating sensors, SendSwitch is just an example, SendTempSensor is another

[szczukot]

But my method dont call UpdateValue or UpdateValueInt. Is faster then SendSwitch etc.
And in SendSwitch etc. we can't set all needed value for type during create device (for example SwitchType for switch)

[gizmocuz]

Thanks for the explanation.
Because of a previous PR merge, there is currently an issue with SQLHelper.cpp that causes this PR not to be merged. The conflict seems small, but maybe better if you take a look at this ?

[gizmocuz]

I also think that those classes should use the default methods to insert new hardware in domoticz, and that is to call the functions from CDomoticzHardwareBase class.
Maybe you could rewrite those, then we do not need this insert device PR ?

[szczukot]

Ok. I will try change it tomorrow.

[gizmocuz]

Sorry, got a merge problem again, is it possible to review it again ?

[gizmocuz]

Thanks for the changes, but do we need those type castings ?

int(pTypeLighting2)
int(sTypeAC)
int(STYPE_Media)
int(m_LEDType)

If possible, please remove them also, makes the code clearer too ;) (I know they are not your castings)

[szczukot]

castings removed

[gizmocuz]

Those classes as mentioned in the issue, should use the default methods to update sensors (with the functions in DomoticzHardware.cpp)

[gizmocuz]

Another possibility was to use

m_mainworker.PushAndWaitRxMessage(this, (const unsigned char *)&gswitch, pDevice->label.c_str(), BatLevel);

See for example the ZWaveBase.cpp

Also when you directly insert/updata data, you are also bypassing all pushers (client/server/mqtt/...) and possible other notification systems

[gizmocuz]

Thanks for the patch ! But most classes should be rewritten to use the internal functions

[emontnemery]

@szczukot, @gizmocuz Maybe this PR needs another review, it caused #2355 and #2375
I also think #2366 might be related.

[gizmocuz]

All hardware classes should use the SendXXXX functions from the basehardware class
This way there is no need to use these dangerous functions at all