search

stanford-oval / thingpedia-common-devices

Thingpedia interface code for commonly used devices
Other
38 stars 28 forks source link

SmartNews needs to track the currently playing news article #318

Open gcampax opened 3 years ago

gcampax commented 3 years ago

So the user can say "stop" and then resume playing news without playing any article that was already played.

Raj-Bapat commented 3 years ago

May I take a stab at this issue? @gcampax

Raj-Bapat commented 3 years ago

hi @gcampax I compared the code for smartnews with that of spotify in order to better understand how to implement a pause method for the smartnews module. I saw in spotify that there is a specific api call that can be used to pause the music, however in smartnews I am unable to find a similar api call. However, I saw that the audio for reading an article is being played from a file streamed from amazon s3; I'm curious as to whether there is a corresponding api calI that can be used to pause the audio file as well. Do you have any thoughts on this?

gcampax commented 3 years ago

Yeah, so the issue here is that SmartNews is built quite differently from Spotify, because SmartNews is a query (get the news), not an action (play music). The SmartNews skill returns the available news and how to present them, but all the heavy lifting around audio is done by the code genie-toolkit code. There is some discussion of the design in https://github.com/stanford-oval/genie-toolkit/pull/552. The existing code handles "stop" to stop playing news. At the same time, the SmartNews skill is not really aware of what news is being played, so it cannot resume playing or filter out news that have been played already.

There are a couple different designs here that could help with this issue. One design is to have some hook to the skill when the audio is being played, so the skill can be aware of what is being played and implement the necessary logic. The other design is to move all this logic into genie-toolkit in the SpeechHandler and AudioController itself. I think the second design is cleaner and perhaps easier to implement, but I'm not 100% sure it would fit all the requirements. So yeah, this issue needs a bit of careful thought first.

Raj-Bapat commented 3 years ago

Hi @gcampax, I am happy to share that I was able to design, code, and validate the stop/resume functionality based on your specifications. I have prepared a user journey and design document for your review. Please let me know what you think of it, and what changes need to be made if necessary.

I will be submitting a pull request shortly for review. After these set of changes, I feel a lot more comfortable working with almond. I look forward to even more future bug fixes and feature requests!

EDIT: I am having a bit of trouble submitting a pull request. The code I edited was in almond-server/node-modules/genie-toolkit, but I am unsure where to submit a request for these files. Could I please know how I am to submit my code?