ebaauw / homebridge-deconz

Homebridge plugin for deCONZ
Apache License 2.0
134 stars 7 forks source link

Fixes a mixup between Consumption and Power services initialization #175

Closed aryelevin closed 12 months ago

aryelevin commented 12 months ago

Fixes a mixup between Consumption and Power services initialization on Accessory index.js file to avoid SyntaxError: totalConsumption: duplicate key on devices with ZHAPower before ZHAConsumption.

ebaauw commented 12 months ago

Already fixed in v1.0.4.

This code wouldn’t fix the issue, but move it to devices where the ZHAConsumption resource comes before the ZHAPower resource.

aryelevin commented 12 months ago

@ebaauw Look on the change before you answer, please. Its not the first time I send you PRs and you reject them so quickly, and then later I see you just done the same thing that I did, don't take it personal, you're doing a great job, and I just want to help you. This change did fixed my ubisys S1-R before your fix on 1.0.4. If look at the code that I changed, I just made sure to load the correct service based on the corresponding service name, you just mixed up the service names and I changed it to be the correct ones.

Thank you a lot - Again

ebaauw commented 12 months ago

I do appreciate your help, and I do apologise for glancing over the change too quickly. I was put off guard a bit by the PR description, and thought you changed the order of:

} else if (params.serviceName === 'Consumption') {

and

 } else if (params.serviceName === 'Power') {

There is no mixup, however; the code combines two resources (ZHAConsumption and ZHAPower) onto the same eve.Consumption service, as this is needed for Eve history. So when evaluating ZHAConsumption, you check whether a service for ZHAPower was already created, and on ZHAPower you check for ZHAConsumption. The TypeError was caused because the service for the second (ZHAConsumption) resource wasn't registered in this.servicesByName and the history code creates a computed totalConsumption for ZHAPower without ZHAConsumption.

aryelevin commented 12 months ago

Oh, OK, Now I understand the code better, maybe a comment there will make it clearer as it placed over many logic if-elses which loads the corresponding service. I don't fully understand how it fixed the error of my S1-R unit, maybe you can tell, I did just discovered this "mixup" (which I understand your point mentioning it serves just another check of having both Power and Consumption services) by debugging the services creation flow using vscode... Well, for now I will reverse my code and will delete the branch I created for it.

Thank you very very much.

ebaauw commented 12 months ago

Its not the first time I send you PRs

I don't see any other PRs by you on any of my repositories? Or did you change usernames on GitHub?

aryelevin commented 12 months ago

It was back in the day on the homebridge-hue days and could be my older GitHub account, I don't remember what was it, but I definitely remember it was funny as a bit later you just did exactly what I PR'ed lol. OK, anyway, I'm happy we're open and discussing anything which is relevant.

aryelevin commented 12 months ago

Oh, well, I realize one of the things. It was even with the homebridge-deconz earlier days, when I wanted to have my ubisys S1 to be used as a HK Switch, and I tried suggesting it to you over Issues page, see #28 for example. Later, you just did it yourself by using Label for ZB switches (Hue Dimmer Switch for example) and using Switch for HK Switches or any other type of light/plug which its serviceName is "Switch". Again, no bad feelings, but I was frustrated to get the feature I had with HB-hue back into HB-deconz. OK, next time lol... Thank you a lot again for the great job.