add GUI + PAUSED to track status

[JarbasAl]

When working on #2660 i realized we also need status to know if playing is using the gui, specially for video/games. This adds 2 more status to the Enum

also see MycroftAI/skill-playback-control#35

[devops-mycroft]

Voight Kampff Integration Test Succeeded (Results)

[devops-mycroft]

Voight Kampff Integration Test Succeeded (Results)

[krisgesling]

Is there any chance we're going to want to add other queuing services in the future?
Eg we had queue (local), queue_audioservice, and now queue_GUI.

It raises the question for me if we anticipate any other services if we should abstract this rather than adding and shifting the ints each time? I can't actually think of any others right now though, so perhaps I'm over thinking it.

Alternatively should we use two digit status ints for all eg:

    QUEUED = 30  # Waiting playback to be handled inside skill
    QUEUED_AUDIOSERVICE = 31  # Waiting playback in audio service
    QUEUED_GUI = 32  # Waiting playback in gui

Especially if we're going to want the service options added to each of the major status codes, we could reserve the x0, x1, x2 codes (or more) for specific services to provide consistency eg

    PLAYING = 20 # local
    PLAYING_AUDIOSERVICE = 21
    PLAYING_GUI = 22 
    QUEUED = 30 # local
    QUEUED_AUDIOSERVICE = 31
    QUEUED_GUI = 32
    PAUSED = 40 # local
    PAUSED_AUDIOSERVICE = 41
    PAUSED_GUI = 42

[forslund]

Is the order here of any importance? It mainly use the IntEnum to be able to serialize the to json without manual conversion. I guess for manually parsing the messages it's easier to see if they're grouped (anything starting with 3 is a queue message, etc)

[krisgesling]

Yeah, you can also use comparison operators with IntEnum's right? So could do things like:

if 30 <= status <= 39:
    self.log.info("I'm queued")

and would mean if we did want to add a new status code in between existing codes, then we don't have to shift all the other codes around.

It may even be beneficial to re-use the majority of the codes for similar status tracking in future systems like a file copy, a robot arm, 3d printer, ... Eg

PRINTING = 20
PRINTING_OCTOPRINT = 21
QUEUED = 30
QUEUED_OCTOPRINT = 31

[JarbasAl]

the whole point of an Enum is that you will never compare with ints directly

if 30 <= status <= 39:
    self.log.info("I'm queued")

should be

if  CPSTrackStatus.QUEUED <= status < CPSTrackStatus.PAUSED:
    self.log.info("I'm queued")

there are no assurances that the int will remain constant

i do like the double digits idea so we can do comparisons like above however

i do not think the printing example really fits here because this is about playback, and the enums should not really need to be expanded again ideally, i do see a few major categories

• audio service handling it
• skill handling it (spotify)
• gui handling it (youtube)
• enclosure handling it (mark1)

maybe add a few more status for the enclosure only? these 4 categories should account for everything

[JarbasAl]

also note that paused etc do not need to differentiate between gui/audio/enclosure, those are "global" status, and are only reverted with a new PLAY status

these are supposed to be further integrated into the audioservice, the playlist handling in particular, but thats a bigger discussion to be had in a follow up PR. Specifically i want to discuss if these status messages should re-order/add to the playlist in audio service or be "read only". Either way the queued and playing will be important to tell apart in the audio service, the pause etc no so much

[krisgesling]

Yeah the other service status's was a bit left field, just thinking at the fringes and it will be good if we have other services like this in the future for them to be consistent. But we really don't need to try and pre-empt all that right now.

So how does the following look:

class CPSTrackStatus(IntEnum):
    DISAMBIGUATION = 10  # not queued for playback, show in GUI
    PLAYING = 20  # CPS is handling playback internally
    PLAYING_AUDIOSERVICE = 21  # Skill forwarded playback to audio service
    PLAYING_SKILL = 22  # Skill forwarded playback to another Skill
    PLAYING_GUI = 23  # Skill forwarded playback to GUI
    PLAYING_ENCLOSURE = 24  # Skill forwarded playback to enclosure
    QUEUED = 30  # Waiting for playback to be handled inside Skill
    QUEUED_AUDIOSERVICE = 31  # Waiting for playback in audio service
    QUEUED_SKILL = 32  # Waiting for playback in another Skill
    QUEUED_GUI = 33  # Waiting for playback in GUI
    QUEUED_ENCLOSURE = 34 # Waiting for playback in enclosure
    PAUSED = 40  # media paused but ready to resume
    STALLED = 60  # stalled state helps to know when to show the buffering ui
    BUFFERING = 61  # In case it's an online source the buffering state or
    END_OF_MEDIA = 90  # helps to know if we want to do autoplay or something

• Is 22 & 32 that what you meant by Skill handling it?
• Should BUFFERING be within the STALLED range?
• This leaves 5x, 7x, 8x clear for future statuses, just in case.

[JarbasAl]

skill handling means that audioservice can not control playback, for example spotify plays with raspotify, this means that the skill itself needs to handle next/prev/pause and so on

list above looks good, will make those changes

Buffering means its loading the data, stalled means its not loading, this was a suggesting by @AIIX so maybe he can explain the rationale better

[krisgesling]

That makes sense, maybe we can tweak the descriptions:

    STALLED = 60  # playback has stalled, reason may be unknown
    BUFFERING = 61  # media is buffering from an external source

[pep8speaks]

Hello @JarbasAl! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2020-08-20 08:16:55 UTC

[devops-mycroft]

Voight Kampff Integration Test Succeeded (Results)

[devops-mycroft]

Voight Kampff Integration Test Succeeded (Results)

[devops-mycroft]

Voight Kampff Integration Test Succeeded (Results)

[forslund]

Gotcha, makes sense if they have some common actions and some specific actions to group them in a range.

[devops-mycroft]

Voight Kampff Integration Test Succeeded (Results)

[devops-mycroft]

Voight Kampff Integration Test Succeeded (Results)