postlund / pyatv

A client library for Apple TV and AirPlay devices
https://pyatv.dev
MIT License
829 stars 87 forks source link

Fix a bug that causes atvscript push_updates to never report the current app #2300

Closed maxileith closed 5 months ago

maxileith commented 6 months ago

Hi,

I have noticed a bug which causes the atvscript push_updates command to never report the App that is currently playing. I believe this bug was caused in https://github.com/postlund/pyatv/pull/1962 which solves https://github.com/postlund/pyatv/issues/1930.

The solution in https://github.com/postlund/pyatv/pull/1962 is supposed to be checking wether or not the App feature is available and only then return it. https://github.com/postlund/pyatv/blob/b35b7ba9bcc93c11754c61ebd99f0cb88d0ce3f2/pyatv/scripts/atvscript.py#L48 However, I think that this is supposed to look like one if these two options:

if not self.atv.features.get_feature(FeatureName.App).state == FeatureState.Unavailable
if not self.atv.features.in_state(FeatureState.Unavailable, FeatureName.App)

The output of atvscript -s xxx.xxx.xxx.xxx push_updates before ...

{
    "result": "success",
    "datetime": "2023-12-04T19:17:02.766358+01:00",
    "hash": "com.apple.avkit.11338.d4e1c618",
    "media_type": "video",
    "device_state": "paused",
    "title": "Fast & Furious 8",
    "artist": null,
    "album": null,
    "genre": null,
    "total_time": 8164,
    "position": 2675,
    "shuffle": "off",
    "repeat": "off",
    "series_name": null,
    "season_number": null,
    "episode_number": null,
    "content_identifier": null,
    "app": null,
    "app_id": null
}

... and after (see attributes app and app_id) ...

{
    "result": "success",
    "datetime": "2023-12-04T19:36:08.452114+01:00",
    "hash": "com.apple.avkit.11338.d4e1c618",
    "media_type": "video",
    "device_state": "paused",
    "title": "Fast & Furious 8",
    "artist": null,
    "album": null,
    "genre": null,
    "total_time": 8164,
    "position": 2691,
    "shuffle": "off",
    "repeat": "off",
    "series_name": null,
    "season_number": null,
    "episode_number": null,
    "content_identifier": null,
    "app": "Netflix",
    "app_id": "com.netflix.Netflix"
}
codecov[bot] commented 6 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (8d883fb) 88.90% compared to head (1e2dcd6) 88.91%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #2300 +/- ## ======================================= Coverage 88.90% 88.91% ======================================= Files 168 168 Lines 11370 11370 ======================================= + Hits 10109 10110 +1 + Misses 1261 1260 -1 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

maxileith commented 5 months ago

I have just rebased the branch ✔️

maxileith commented 5 months ago

Okay, I will update the PR.

maxileith commented 5 months ago

I believe we should do this here:

            if not self.atv.features.in_state(FeatureName.App, FeatureState.Unavailable

should be if not self.atv.features.in_state(FeatureState.Unavailable, FeatureName.App)

postlund commented 5 months ago

Great! 👍

maxileith commented 5 months ago

Is there anything left to do in order to get this merged and released? :)

I am willing to help where I can.