-
-
Notifications
You must be signed in to change notification settings - Fork 218
fix scheduler: scheduled playlist never taking over from manual playlist because of priority issue #2528
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
base: master
Are you sure you want to change the base?
Conversation
…ually started playlists because manual playlists always had a lower (higher priority) value
|
Closing until I understand what Remote Falcon does. Will do further testing. |
|
OK, I tested Remote Falcon and it works as expected with this change. |
|
I'm not sure this is the behaviour we want though - think the idea was that manually triggered playlist would take preference over scheduled |
|
I assumed based on the comments in the code that were already there that scheduled should take priority over manual. I think users sometimes start manual things and then expect their schedule to take over; the user I worked with on FB did and they said that's how it worked in the past and was a major frustration considering the change in Nov. In the current scenario, how would a scheduled playlist ever start when a manual one is ran and maybe accidentally put on repeat? |
|
So, I'm the user here. My pre show playlist gets kicked off via MQTT or REST so it can be played from a few minutes past sunset until my show starts via the schedule. This has worked as expected for years and even seems to work occasionally on v9.x, though I'm not sure how that would be. Why would it have changed? Seems like some sort of No Override option would be a better solution than a hard coded 'run forever'. |
|
Its not as simple as a schedule. I have a number of things being down prior to the manual playlist starting (power ups, pings, lights, etc) so I don't want it to just kick off arbitrarily. I think the bigger question is why a breaking change was introduced when it's been working for years. Was this in response to some issue? Shouldn't a change like this have been set up as an option? |
|
Even using the scheduler for that, I think there is a gap in logic here to make sure people know what's happening. Maybe there should be a global option in scheduler that is something to the effect of "Schedules will start over manually started playlists". The scheduler has the whole concept of priority but manual sort of blows that away. The additional logging would also be helpful. |
|
Not at my machine tonight but pretty sure I made the default priority -1 on starting playlists but the function allows you to pass a higher priority so you can force a playlist to be highrer than the scheduled ones if required, will look properly tomorrow to see if its just a case of exposing the priority setting in the api calls |
|
Have had a chat with the other dev's on this and implemented a new approach with brings back the old behaviour but now allows a new set of settings to enable the user to let manually started items be protected from the scheduler messing them up - works both from the UI and api/mqtt etc.. also added UI warnings if a scheduled item is skipped due to this setting - please test out and confirm you have the behaviour you want and we will close this PR as superseeded by the new functionality |

I worked with a user on Facebook why their scheduler was never starting. After some debugging and back and forth they would have a manual playlist running and the scheduled playlist would just dump log entries over and over (after turning on higher level logging):
Root Cause
Commit 7dc7962 added priority checking but used a value that would make manual playlists always take precedence over scheduled playlists.
Manual playlists were set to -1 while scheduled playlists use entry index (0-99). Since -1 is lower (higher priority), scheduled playlists could never take priority over manual ones.
I have set the default value to an artifically high level to overcome. I have also added some debug logging to make what is happening more clear in logs.
To replicate on Master
After Fix