Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remember listeners on AccessoryInformation if returned by getServices #2169

Closed
wants to merge 1 commit into from
Closed

Conversation

mrose17
Copy link

@mrose17 mrose17 commented Mar 3, 2019

if getServices returns a Service.AccessoryInformation instance, the existing code copies the value but not the listeners. In some cases (e.g., in https://github.com/homespun/homebridge-accessory-apcupsd/blob/master/index.js#L344), it isn't "convenient" to retrieve model and serial number information synchronously (cf., #697) .

The proposed code copies any listeners in the returned Service.AccessoryInformation instance so as to defer binding of the value until "convenient"

Enjoy!

if `getServices` returns a Service.AccessoryInformation instance, the  existing code copies the `value` but not the `listeners`. In some cases (e.g., in https://github.com/homespun/homebridge-accessory-apcupsd), it isn't "convenient" to retrieve model and serial number information synchronously (cf., #697) .

The proposed code copies any listeners in the returned Service.AccessoryInformation instance so as to defer binding of the value until "convenient"

Enjoy!
mrose17 added a commit to homespun/homebridge-accessory-apcupsd that referenced this pull request Mar 3, 2019
Prepare for possible merge of homebridge/homebridge#2169

Add apcupsd tutorial to README
@tobekas
Copy link

tobekas commented Jun 25, 2019

Name, Model, Manufacurer and SerialNumber must be persistent during the lifetime of the accessory. (by spec)
This may be the reason why these values are copied... But since Firmware/Hardware Revision may chance, this issue still has its raison d’être. See also: #2207

var srcCharacteristic = srcService.getCharacteristic(characteristicUUID);
var dstCharacteristic = dstService.getCharacteristic(characteristicUUID);

if (!srcCharacteristic) return;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will always return false, as "getCharacteristic" will create the characteristic if it is not present.

@stale stale bot removed the stale label Apr 3, 2020
@Supereg
Copy link
Member

Supereg commented Apr 3, 2020

This PR was pretty much replaced by #2472

But we could adopt the way listeners get replaced. But maybe we should add the code doing the replacements to hap-nodejs? So we can better track any possible changes made to event types.

@afharo
Copy link
Contributor

afharo commented Apr 3, 2020

This PR was pretty much replaced by #2472

I don't think that's entirely true. We might need to update this PR based on the changes in #2472 to match the new generic way of looping through all the services but the UUID one.
But I don't think it's entirely replaced by it since the listeners are not passed through, which is the original intention of this PR.

@donavanbecker
Copy link
Contributor

closing in favor of #2476

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants