Skip to content
This repository was archived by the owner on Sep 8, 2024. It is now read-only.

Feat/cps media type #2660

Closed
wants to merge 10 commits into from
Closed

Feat/cps media type #2660

wants to merge 10 commits into from

Conversation

JarbasAl
Copy link
Contributor

@JarbasAl JarbasAl commented Aug 13, 2020 •
edited
Loading

possible solution for #2658

To test update CommonPlay skill MycroftAI/skill-playback-control#32

install https://github.com/JarbasSkills/skill-hppodcraft/tree/cps_mediatype and verify

  • "read the call of cthulhu" uses common play with media type audiobook,
  • "play lovecraft podcast" should use podcast media type
  • "play {story_name}" should trigger with generic media type

install https://github.com/JarbasSkills/skil-old-world-radio/tree/cps_media_type and verify

  • only triggers with media type radio
  • "play vintage radio", "play old world radio"
  • "play some radio" should trigger with low confidence

install MycroftAI/skill-npr-news#90

  • verify "play the news" triggers default feed with type news
  • verify "play {station_name} news" triggers news station with type news
  • "play {station_name}" should trigger with type generic

install https://github.com/JarbasSkills/skill-epic-horror-theatre/tree/feat/cps_media

  • "read innsmouth" uses common play with media type audiobook,
  • "read color out of space audio drama" uses common play with media type audiobook,
  • "play the mountains of madness" should trigger with generic media type

@JarbasAl JarbasAl added the Type: Enhancement - proposed New proposal for a feature that is not currently a priority on the roadmap. label Aug 13, 2020
@pep8speaks
Copy link

pep8speaks commented Aug 13, 2020 •
edited
Loading

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-09-20 21:55:32 UTC

@devops-mycroft devops-mycroft added the CLA: Yes Contributor License Agreement exists (see https://github.com/MycroftAI/contributors) label Aug 13, 2020
@JarbasAl JarbasAl mentioned this pull request Aug 13, 2020
Closed
@JarbasAl JarbasAl force-pushed the feat/cps_media_type branch from aaa4bd6 to 3924942 Compare August 13, 2020 05:42
@devops-mycroft
Copy link

Voight Kampff Integration Test Failed (Results)

1 similar comment
@devops-mycroft
Copy link

Voight Kampff Integration Test Failed (Results)

forslund
forslund reviewed Aug 13, 2020
View reviewed changes
Copy link
Collaborator

@forslund forslund left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks very nice imo (haven't tried it yet). Did leave a comment on the possibility to use inspect/signature instead of try/except

@devops-mycroft
Copy link

Voight Kampff Integration Test Failed (Results)

@JarbasAl
Copy link
Contributor Author

install https://github.com/JarbasSkills/skil-old-world-radio/tree/cps_media_type and verify

  • only triggers with media type radio
  • "play vintage radio", "play old world radio"

@JarbasAl JarbasAl mentioned this pull request Aug 18, 2020
Closed
@devops-mycroft
Copy link

Voight Kampff Integration Test Succeeded (Results)

1 similar comment
@devops-mycroft
Copy link

Voight Kampff Integration Test Succeeded (Results)

This was referenced Aug 18, 2020
Merged
Merged
@JarbasAl JarbasAl requested review from forslund and AIIX August 18, 2020 22:22
@JarbasAl JarbasAl mentioned this pull request Aug 19, 2020
Closed
@devops-mycroft
Copy link

Voight Kampff Integration Test Succeeded (Results)

@JarbasAl
Copy link
Contributor Author

JarbasAl commented Aug 27, 2020 •
edited
Loading

for video playback examples check
https://github.com/JarbasSkills/skill-dagon/tree/cps_media and https://github.com/JarbasSkills/skill-euronews/tree/feat/cps_media

sets PLAYING_GUI status from #2674 and handles video match type

@JarbasAl JarbasAl mentioned this pull request Aug 27, 2020
Merged
@JarbasAl JarbasAl force-pushed the feat/cps_media_type branch from b082a7f to 9eeeb32 Compare August 27, 2020 19:58
@devops-mycroft
Copy link

Voight Kampff Integration Test Succeeded (Results)

@devops-mycroft
Copy link

Voight Kampff Integration Test Succeeded (Results)

@devops-mycroft
Copy link

Voight Kampff Integration Test Succeeded (Results)

@devops-mycroft
Copy link

Voight Kampff Integration Test Failed (Results)

@devops-mycroft
Copy link

Voight Kampff Integration Test Failed (Results)

@krisgesling krisgesling added the Status: Blocked PR is blocked by some dependency, eg another PR that needs to be merged first. label Nov 13, 2020
@JarbasAl JarbasAl closed this Dec 2, 2020
This was referenced Dec 22, 2020
Closed
Merged
@JarbasAl JarbasAl mentioned this pull request Mar 8, 2021
Closed
@AIIX AIIX mentioned this pull request Jun 3, 2021
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@forslund forslund Awaiting requested review from forslund

@AIIX AIIX Awaiting requested review from AIIX

Assignees
No one assigned
Labels
CLA: Yes Contributor License Agreement exists (see https://github.com/MycroftAI/contributors) Status: Blocked PR is blocked by some dependency, eg another PR that needs to be merged first. Type: Enhancement - proposed New proposal for a feature that is not currently a priority on the roadmap.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@JarbasAl @pep8speaks @devops-mycroft @forslund @krisgesling