cflurin / homebridge-mqtt

Homebridge-mqtt is a Plugin for Homebridge.
Apache License 2.0
229 stars 38 forks source link

Properties option for get all accessories #92

Closed roscoe81 closed 4 years ago

roscoe81 commented 4 years ago

Added the ability to also capture characteristic properties of each accessory with the homebridge/to/get topic. This is achieved by using an additional {"name": "*_props"} payload to avoid backwards compatibility issues.

bohtho commented 4 years ago

Nice PR!

Wouldnt it be more in style with the current API if properties are captured by sending {“name”:”*”} to homebridge/to/get/properties ? Like how homebridge/to/add/service was added after homebridge/to/add ?

EDIT: I am using topic_type multiple, so I now realise it might interfere if topic_type is single and an accessory is called “properties”. But in that case, what about homebridge/to/get/_properties (in the case of topic_type multiple) ?

Anyway, I wouldn’t mind a potentially "breaking" change in Homebridge/to/get with {“name”:”*”) for this. I assume people parse the response anyway. It shpuld't be breaking as you are adding "root" branches to the JSON, and not moving or changing anything existing.

roscoe81 commented 4 years ago

Many thanks for your comment @bohtho. I'm OK with it being implemented via a new mqtt topic, instead of a new payload, if that's the final decision. I chose to do the PR via a new payload because I felt that it's delivering a superset of the config that's currently delivered by homebridge/to/get (i.e. it's delivering all that homebridge/to/get {"name": ""} delivers, plus some additional information) . Using your comparison with homebridge/to/add, my view was that homebridge/to/add/service is managing a subset of what is managed by homebridge/to/add and therefore I'd risk inconsistency if I used homebridge/to/get/properties. I guess that it's just a different perspective. Like you, I would have been comfortable with a potentially "breaking" change via {"name: ""}, but I didn't want to risk it for the broader user community.