-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
feat: improve OTA #30566
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
feat: improve OTA #30566
Changes from all commits
f767f2f
cf1fd70
f9c7b75
6aa0fe0
a792c7a
dc37b69
9e60626
cd3cbb6
b641f73
4cdad1f
1535481
eb98d7a
27bb8aa
dc155b4
5d40f94
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -427,6 +427,7 @@ export class HomeAssistant extends Extension { | |
| this.eventBus.onGroupMembersChanged(this, this.onGroupMembersChanged); | ||
| this.eventBus.onDeviceAnnounce(this, this.onZigbeeEvent); | ||
| this.eventBus.onDeviceJoined(this, this.onZigbeeEvent); | ||
| // TODO: this is triggering for any `data.status`? | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it makes sense to do this always, similar to https://github.com/Koenkk/zigbee2mqtt/pull/30566/changes#r2722613265 the error might be not critical and maybe all info needed to identify the device has been received, thus it can discovered. |
||
| this.eventBus.onDeviceInterview(this, this.onZigbeeEvent); | ||
| this.eventBus.onDeviceMessage(this, this.onZigbeeEvent); | ||
| this.eventBus.onScenesChanged(this, this.onScenesChanged); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -37,6 +37,8 @@ export default class OnEvent extends Extension { | |
| } | ||
| }); | ||
| this.eventBus.onDeviceInterview(this, async (data) => { | ||
| // TODO: this is triggering for any `data.status`, any use outside `successful`? | ||
| // ZHC definition would skip from `device.definition?` but OnEvent from ZHC index wouldn't => triple triggering? | ||
|
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Interview failed doesn't (always) mean there is no definition, so I think this is fine.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The problem I was thinking for this and all above is the fact
Owner
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It depends a bit wether we also want to trigger the
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I think it's worth a look in a follow-up PR, same for the other 2 in HA and configure extensions, should de-dupe calls a bit. |
||
| await this.callOnEvent(data.device, {type: "deviceInterview", data: {...this.#getOnEventBaseData(data.device), status: data.status}}); | ||
| }); | ||
| this.eventBus.onDeviceAnnounce(this, async (data) => { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that even when a device interview fails, it's worth configuring it as it might be due to a non crucial error and then the configure makes the device at least partially working.