forslund / spotify-skill

Mycroft Skill to control spotify using the Spotify Connect API
Apache License 2.0
71 stars 38 forks source link

Spotify devices do not match due to case-sensitivity #160

Closed rickoooooo closed 3 years ago

rickoooooo commented 3 years ago

My Mycroft instance has been unable to transfer Spotify playback to any of my Windows-based devices. I discovered this seems to be a case-sensitivity problem. One of my Windows desktop systems has the hostname "officepc". However, Spotify names the device "OFFICEPC". Notice it's been converted to all uppercase. If I use the "list spotify devices" intent, the skill will list all of my devices just fine. However, when I try to "transfer playback to officepc", the skill tells me it can't find a device named "officepc".

spotify_1

I debugged this a bit and found that the match_one() function call within device_by_name() function appears to be case-sensitive. Mycroft converts all of my voice input to lowercase (OFFICEPC becomes officepc), but the Spotify skill sees my device name as "OFFICEPC". Therefore, match_one() never matches.

spotify_2

I've made a simple modification in my local code to convert the device names in the devices list to lowercase and this seems to get around the problem. I'm not sure if this could introduce any other problems, like if it's possible to have a Spotify devices named OFFICEPC and a second device named officepc, in which case this would confuse things. I think Mycroft always converts voice input to lowercase though, so this would likely be a problem in any case.

spotify_3

Original code: devices_by_name = {d['name']: d for d in devices}

Updated code: devices_by_name = {d['name'].lower(): d for d in devices}

forslund commented 3 years ago

Hi,

thanks for digging into this. it sounds like a simple way to improve the functionality so I'm all for this.

If conflicts are an issue in the future we should add some sort of disambiguation. Like the question did you mean officepc or uppercase OFFICEPC...

but let's cross that bridge when we get to it...

I'd be happy to accept a pull request

forslund commented 3 years ago

Resolved by #161