sebbo2002 / node-pyatv

A lightweight node.js wrapper around pyatv…
MIT License
26 stars 2 forks source link

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

Closed maxileith closed 6 months ago

maxileith commented 6 months ago

Hi @sebbo2002 ,

this Issue is related to #291. An user of my plugin @rvetere has reported the issue https://github.com/maxileith/homebridge-appletv-enhanced/issues/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: https://github.com/sebbo2002/node-pyatv/blob/60a414daeaac29c53254611ef25a54dbf671a720/src/lib/device-events.ts#L38-L52 I think it should be handled like power_state or volume

sebbo2002 commented 6 months ago

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...

maxileith commented 6 months ago

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

https://github.com/sebbo2002/node-pyatv/blob/60a414daeaac29c53254611ef25a54dbf671a720/src/lib/tools.ts#L237

Then the following method will set all states to null.

https://github.com/sebbo2002/node-pyatv/blob/60a414daeaac29c53254611ef25a54dbf671a720/src/lib/device-events.ts#L38

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

sebbo2002 commented 6 months ago

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 commented 6 months ago

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

maxileith commented 6 months ago

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 commented 6 months ago

300 merged in favor of #299.

maxileith commented 6 months ago

@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 commented 6 months ago

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

The release is available on:

Your semantic-release bot :package::rocket:

sebbo2002 commented 5 months ago

:tada: This issue has been resolved in version 7.3.0 :tada:

The release is available on:

Your semantic-release bot :package::rocket: