AlarmClock.prototype.PatchAlarm does not support multiple players

[magnlund]

When trying to manage an alarm (e.g disable/enable) through alarmService.SetAlarm error is being raised telling that ID cannot be found. The error is raised in patchAlarm function located in /lib/services/AlarmClock.js

Expected Behavior

It should be possible to manage alarms even in a multiroom speaker setup

Current Behavior

Error message "Alarm with id not found" is being raised

Possible Solution

Modify line 106 in AlarmClock.js to support multiroom speaker setup. When having multiple speaker a list is generated by Sonos. The current code is likely based on one speaker setup

Sample code or executed example

I replaced line 106 with: let targetAlarm = result.Alarms.find((Alarms) => { return result.Alarms[0].ID === id })

but only as a quick fix due to the fact that I only have 1 active alarm in my setup

Versions (and Environment)

Node version: 8.9.3 node-sonos version: 1.10.1 OS: Windows 10 for test, Linux om Raspberry PI in Prod

[bencevans]

You may well have hit the actual fix there. Due to the XML parsing it's likely the ID will be at [0].ID as the parser isn't sure if there's additional elements it defaults to placing them in arrays.

[svrooij]

The code to update alarms works perfect here, but it also accepted a number as ID parameter. the === operator doesn't think 17 is the same as '17'. And Sonos needs the ID specified as a string for it to work.

The above PR also adds a test for switching the alarms.

[wwwizzarrdry]

Nice find @magnlund

[magnlund]

Correct. Sonos Seems to require a number expressed in as a string

Skickat från ProtonMail mobile

-------- Originalmeddelande -------- På 3 maj 2019 17:11, Stephan van Rooij skrev:

The code to update alarms works perfect here, but it also accepted a number as ID parameter. the === operator doesn't think 17 is the same as '17'. And Sonos needs the ID specified as a string for it to work.

The above PR also adds a test for switching the alarms.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or mute the thread.

[bencevans]

:tada: This issue has been resolved in version 1.11.0 :tada:

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot :package::rocket:

[magnlund]

So Great 😀 Thanks

Skickat från ProtonMail mobile

-------- Originalmeddelande -------- På 6 maj 2019 21:18, Ben Evans skrev:

🎉 This issue has been resolved in version 1.11.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.