MycroftAI / skill-npr-news

Mycroft AI official News Skill, providing the latest news report from your favorite broadcast.
https://mycroft.ai/skills
Apache License 2.0
9 stars 42 forks source link

Assumption that first link of first entry is audio is not true #17

Closed zostay closed 5 years ago

zostay commented 6 years ago

https://github.com/MycroftAI/skill-npr-news/blob/c256291c6fa5d38b301419fd6ae9f727d04eac50/__init__.py#L68

This assumes that the first link of a podcast RSS feed is the audio. This may work for the items in the drop down, but is not true of all.

For example,

https://albertmohler.com/category/the-briefing/feed/

That feed does not play because the first link is not the audio.

I suggest that a better heuristic should be employed. If there's only one link, the first is fine. If there's more than one link, then check for rel=enclosure and/or type=audio/mpeg and similar to try and make sure the correct link is picked.

I was going to submit this as a PR, but my Mark 1's SD Card bit the dust again, so I need to deal with that problem before I can attempt a fix.

Cheers.

forslund commented 6 years ago

Thanks for reporting. See PR #13, which does something similar. I've been busy with this release but that should be merged soon, if you have suggestions on improvement on it add a comment to the PR.

forslund commented 5 years ago

I believe this has been addressed now. Are you ok with closing this issue?

juhi24 commented 5 years ago

I finally installed the latest Mycroft and tested this. Works great with the link given in #13 and the example link of the OP, so this seems to be fixed in #33. I think this issue can be closed now.

forslund commented 5 years ago

Right you are. Thanks for your work on this. Closing.