-
-
Notifications
You must be signed in to change notification settings - Fork 18
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Beta test and vote: Notification queue #119
Comments
I have already a solution in place (currently based on node-sonos):
and I leave the queuing of notifications in the responsibility of the user. In Node-RED you can do that with a simple "Queue" node in front of a player. So init, refresh, reset, and the queuing itself of notification is in his responsibility :-) Anyway: The issue with "notification queuing" is that usually there are several people in a household and they (can) use different apps to send commands to the SONOS player. So the system can be easily messed up. Bottom line: PlayNotification would be my preferred solution as I might reuse getState/restoreStore methods and replace my group.create/play.snap. |
Closing stale issue, lets keep the new notification way in the code for now. |
@svrooij just as a short update from my side: I retried using your solution and can't run it in my enviroment due to the following:
I can't set timeout because, I would need to set it considering the other items in queue. Shall I take a look to implement the |
I guess the PlayTtsTwo doesn't work correctly because I rushed it. This should be modified to use the send notification method. public async PlayTTSTwo(options: PlayTtsOptions): Promise<boolean> {
this.debug('PlayTTSTwo(%o)', options);
const notificationOptions = await TtsHelper.TtsOptionsToNotification(options);
return await this.PlayNotification(notificationOptions);
} I guess if you fix this method, the regular notification method wouldn't need any changes. |
Since 2.4.0-beta.2 we support a second implementation of notification queues #89
I added both so everybody can check them out. This issue described the differences and I'm curious to what version works best.
To just vote for an implementation, react with:
But it's more helpful to reply and explain what you've found.
Pros and cons
The list of pros and cons might not be complete, please reply and I'll edit this issue
Pros and cons for PlayNotification
Pros:
GetState
andRestoreState
, to build your own app around getting and restoring state.Cons:
Pros and cons for PlayNotificationTwo
Pros:
Cons:
Details
PlayNotification
This method uses the calling thread of the first
PlayNotification
call. The first call will return when all notifications have played. The calls while currently playing a notification will return when that notification is added to the array and will be played in the thread of the first call.Each notification can still use the timeout (in seconds) to stop the playback if the event isn't received or if you're playing a stream.
This implementation uses about 120 lines of code and the complexity is moderate.
node-sonos-ts/src/sonos-device.ts
Lines 351 to 469 in 94d2d5e
node-sonos-ts/src/sonos-device.ts
Lines 548 to 583 in 94d2d5e
PlayNotificationTwo
This implementation uses several timers to disconnect the thread, but each call to
PlayNotificationTwo
returns when that specific notification has played.This implementation uses about 250 lines of code and the complexity is high.
node-sonos-ts/src/sonos-device.ts
Lines 485 to 497 in 94d2d5e
node-sonos-ts/src/sonos-device.ts
Lines 1107 to 1392 in 94d2d5e
node-sonos-ts/src/models/notificationQueue.ts
Lines 1 to 93 in 94d2d5e
The text was updated successfully, but these errors were encountered: