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

output_devices in response of atvscript push_updates resulting in faulty state #295

Closed
maxileith opened this issue Jan 11, 2024 · 9 comments
Assignees
Labels
bug Something isn't working enhancement New feature or request released on @next released

Comments

@maxileith
Copy link
Contributor

maxileith commented Jan 11, 2024

Hi @sebbo2002 ,

this Issue is related to #291. An user of my plugin @rvetere has reported the issue maxileith/homebridge-appletv-enhanced#155. After some investigation the root cause is similar to the on reported in #291. Below is the response of atvscript push_updates

{"result": "success", "datetime": "2024-01-11T19:45:57.122827+01:00", "power_state": "on"}
{"result": "success", "datetime": "2024-01-11T19:45:57.123138+01:00", "output_devices": [{"name": "Apple TV", "identifier": "D4ED48E4-4C88-47C0-B05C-077747F85C5D"}]}
{"result": "success", "datetime": "2024-01-11T19:45:57.125393+01:00", "hash": "ca496c14642c78af6dd4250191fe175f6dafd72b4c33bcbab43c454aae051da1", "media_type": "unknown", "device_state": "idle", "title": null, "artist": null, "album": null, "genre": null, "total_time": null, "position": null, "shuffle": "off", "repeat": "off", "series_name": null, "season_number": null, "episode_number": null, "content_identifier": null, "app": null, "app_id": null}
{"result": "success", "datetime": "2024-01-11T19:45:57.126692+01:00", "volume": 50.0}

… which does include a line with the unexpected attribute output_devices which I do not think is correctly handled here:

applyStateAndEmitEvents(newState: NodePyATVState): void {
let keys = Object.keys(this.state);
if('power_state' in newState && newState.power_state) {
keys = ['power_state'];
}
if('focus_state' in newState && newState.focus_state) {
keys = ['focus_state'];
}
// Volume events don't hold the complete state…
// see https://github.com/sebbo2002/node-pyatv/pull/291
if('volume' in newState && newState.volume) {
keys = ['volume'];
}
I think it should be handled like power_state or volume

@sebbo2002
Copy link
Owner

The field output_devices is currently not implemented and is therefore neither mapped in events nor in NodePyATVState. I'll put it on my todo list, I'm assuming to have something by the beginning to mid of next week...

@sebbo2002 sebbo2002 self-assigned this Jan 12, 2024
@sebbo2002 sebbo2002 added the enhancement New feature or request label Jan 12, 2024
@maxileith
Copy link
Contributor Author

The problem is, that this function will return its default when passing an Object just with output_devices, so null for every attribute.

export function parseState(input: NodePyATVInternalState, id: string, options: NodePyATVInstanceOptions): NodePyATVState {

Then the following method will set all states to null.

applyStateAndEmitEvents(newState: NodePyATVState): void {

So I would consider this a bug, not an enhancement.

@sebbo2002
Copy link
Owner

Oh. I'm glad you wrote again. I somehow didn't realize that the first time I read it. Yes, of course I should fix that also, so that applyStateAndEmitEvents does not trigger any events if everything is set to null. That would solve the problem, right?

@sebbo2002 sebbo2002 added the bug Something isn't working label Jan 12, 2024
sebbo2002 added a commit that referenced this issue Jan 15, 2024
@sebbo2002 sebbo2002 mentioned this issue Jan 15, 2024
@sebbo2002
Copy link
Owner

@maxileith Both should be fixed with 7.3.0-develop.1. Can you confirm that the fix works?

@maxileith
Copy link
Contributor Author

maxileith commented Jan 15, 2024

I think there is a change required, see #299. Nevertheless, I already want to thank you for the fast replies and fixes in this project.

@sebbo2002
Copy link
Owner

#300 merged in favor of #299.

@maxileith
Copy link
Contributor Author

@sebbo2002 I left a comment in #300. You may want to take a look at this. However, this feature should work either way for me. I will release a beta of my plugin and ask the author of the bug reported if it is working for him 👍🏻

@sebbo2002
Copy link
Owner

🎉 This issue has been resolved in version 7.3.0-develop.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

github-actions bot pushed a commit that referenced this issue Jan 30, 2024
# [7.3.0](v7.2.1...v7.3.0) (2024-01-30)

### Bug Fixes

* **core:** fix typo getParameters ([e28b86b](e28b86b))
* **core:** fix unintentional event trigger ([c4bd9a5](c4bd9a5))
* Do not reset state to initial default state on unsupported messages ([876c9b4](876c9b4)), closes [#295](#295)
* **Types:** Fix `NodePyATVFocusState` typo ([cc96a83](cc96a83))

### Features

* Add `outputDevices` Support ([cf194fd](cf194fd)), closes [#295](#295)
@sebbo2002
Copy link
Owner

🎉 This issue has been resolved in version 7.3.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request released on @next released
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants