genielabs / HomeGenie

HomeGenie, the programmable automation intelligence
https://homegenie.it
GNU General Public License v3.0
392 stars 155 forks source link

HG doesn't use custom device handler after startup #78

Closed Bounz closed 9 years ago

Bounz commented 9 years ago

HG doesn't use custom device handler after startup. For example I have Z-Wave.Me Floor Thermostat and there is a custom device handler for this device. After first discovery HG doesn't ask devices for manufacturer specific info and uses generic handlers. I go to Groups and Modules and requests manufacturer specific info from node. At this point HG finds that it has a device specific handler and starts use it. Well, let's save configuration and restart HG. Now, if we open device properties, we can see that it have "ZWaveNode.DeviceHandler" and "ZWaveNode.ManufacturerSpecific" fields filled correctly, but obviously generic Thermostat handler is used. I've to go to module settings and request manufacturer specific info again.

I'll try to fix this.

Bounz commented 9 years ago

Found this line commented in Controller.cs (https://github.com/genielabs/HomeGenie/blob/master/MigFiles/SupportLibraries/ZWaveLib/Devices/Controller.cs#L647):

//znode.ManufacturerSpecific_Get();

What was the reason of commenting this line?

genemars commented 9 years ago

The reason is that I didn't test it. Also don't know what happen with battery powered devices since these won't respond to the command unless they are awake. I believe that the manufacturer specific data should be cached in the module properties, so no need to ask for it everytime. Isn't it? (perhaps this module property is not saved if HG is stopped suddenly and so without saving data to the modules.xml file).

Bounz commented 9 years ago

Hi, Genie. I'm sorry for the late answer - I'm far away from home now. As I see the problem is in the order of initializing z-wave subsystem: after creating z-wave port HG starts z-wave discovery procedure. During discovery HG doesn't ask nodes for manufacturer specific data and assign them generic handlers. I didn't found code where cached manufacturer specific data is used during initialization. I think the procedure of z-wave initialization should be changed:

  1. Start discovery.
  2. For every node discovery check if this node is already in saved configuration. If yes then assign device handler defined in module properties. If no - then it's new device in network and we should add it it.
  3. Discovery complete.
genemars commented 9 years ago

@Bounz read this http://www.homegenie.it/forum/index.php?topic=626.msg3561 it might contains useful info.

Bounz commented 9 years ago

@genielabs, thanks!= I'll dig more inside this.

junknown commented 9 years ago

@Bounz @genielabs if i have some free time this week i can try to fix it ... my idea was to provide a generic method ( fake command maybe..) to pass module's property to corresponding interface nodes. So we can expand it saving node properties we need in module configuration file ( device handler is already saved , we don't need to query for it anymore , expecially for sleeping battery sensors).

genemars commented 9 years ago

http://sourceforge.net/p/homegenie/code/332/tree/trunk/HomeGenie/Service/HomeGenieService.cs#l969

this is how it was done in a very old release of HG. But I deprecated it when [re]moving all protocol-specific stuff off from HG service code.

Bounz commented 9 years ago

Well, because specific MIG interfaces realizations are coupled with related MIG support libraries (and classes representing real devices) I think one possible solution is to pass modules configuration stored in .xml to specified MIG interface as a constructor parameter (https://github.com/genielabs/HomeGenie/blob/master/MigFiles/MIG/MIGService.cs#L185). Then interface could use it to restore its previous state. For example, ZWave interface could add ZWaveNodes to controller before initial discovery and then add or remove some nodes if they were added or removed from ZWave network since last run. One additional problem is how HG stores its configuration: modules are separated from interfaces. But we can use Module.Domain property to filter modules related to specific interfaces. What do you think?

junknown commented 9 years ago

It's a good solution but i'm afraid that we're mixing the module's configurations with node's interface's configurations.... What do you think about handling a zwave.xml config file with node's list and device handler. It can be read on interface connect method, in synchoronus way. During discovery we can check if node already exists and make a merge. On "discovery end" event we can handle to resave the configuration file...

Bounz commented 9 years ago

As I noticed from code MIG interfaces are there exacltly for that, they are a tier between MIG modules and hardware. Look at https://github.com/genielabs/HomeGenie/blob/master/MigFiles/MIG/Interfaces/HomeAutomation/ZWave.cs#L247, GetModules() works directly with controller and ZWaveNodes to present them as InterfaceModules. My offer is based on idea of minimizing impact on existing code. Your idea is also interesting: each interface handles it's own configuration. @genielabs what are you thoughts?

genemars commented 9 years ago

good places to put the code could be either the ZWaveLib or the MIG Interface. HomeGenie itself should be preserved to keep its "agnostic" behaviour. Perhaps putting the new code in the ZWaveLib is a good thing since it will make the library fix the issue also when used in some other project. Just let's think more about this before implementing the code. A data file (like zwave.xml) seems to be a due solution where to persist nodes informations.

genemars commented 9 years ago

please @Bounz, @junknown make some testing =)

Bounz commented 9 years ago

Great! First tests were successful! With the new version of HG I had to retrieve nodes manufacturer specific info one more time and after restart handlers were restored. @genielabs Gene, thank you very much! You did a great job and I believe that this fix will greatly improve system's consistence.

junknown commented 9 years ago

Yes , now it works... i just updated Hg and restarted it and everything seems work perfectly... Thanks @genielabs

genemars commented 9 years ago

I noticed there's a little new bug regarding module parameter saving, but that's another story =) btw it is already fixed in the next coming commit. thanks @Bounz @junknown for testing!