mopidy-alarmclock icon indicating copy to clipboard operation
mopidy-alarmclock copied to clipboard

Support for multiple alarms

Open rgov opened this issue 8 years ago • 15 comments

I apologize for a pull request that is a heavy refactor. These changes add support for multiple alarms, and generally might make it easier to implement new features in the future.

  • Introduces an Alarm model which contains settings for a single alarm.
  • Allows each alarm to be enabled or disabled.
  • Changes the web UI to allow the creation, deletion, and configuration of multiple alarms.
  • Refactors the AlarmManager to greatly simplify the way the idle timer works.
  • Rewrites the way the volume fade works.
  • Lays the groundwork for weekday settings on alarms.

Not yet done:

  • I did not implement any test cases, and probably broke all of the old ones. Coveralls is going to be unhappy.
  • New alarms do not get configured with the default settings.
  • Alarms are not persisted to disk. (This is not a regression but should be high priority.)
  • The UI could use a coat of paint.
  • The use of timers could be made much more efficient.

rgov avatar Feb 27 '17 02:02 rgov

Thank you very much for the pull request! 👍 I will some review (and possibly additions) of the code when I will have the time (hopefully this or next weekend).

I guess that the goal of multiple alarms is to have something similar to the alarm clocks of smartphones... Is this our common vision of this feature?

Another thing which I would like to add is that stability is one of priorities of this application, so I guess we can make the refactoring in some separate branch and after initial field testing merge it in develop.

DavisNT avatar Feb 28 '17 22:02 DavisNT

I am building a "radio" that will switch between Pandora stations for different genres at different times of day. Mopidy-AlarmClock was a good starting point for developing this because it can play a playlist at a given time of day, but I needed to be able to set more than one alarm.

For users who only need one alarm, it will still serve their purposes with this feature, but it allows Mopidy-AlarmClock to be used for more complex scheduling.

rgov avatar Mar 01 '17 01:03 rgov

Another thing which I would like to add is that stability is one of priorities of this application, so I guess we can make the refactoring in some separate branch and after initial field testing merge it in develop.

Isn't develop supposed to be the unstable branch?

rgov avatar Mar 01 '17 01:03 rgov

@jcass77: It deserves consideration. Would you want to set a separate alarm for "stop playing at this time" (possibly as another option in the Playlist selector), or would you want to set an alarm with "play this now, then stop playing after x minutes"? You should file a new issue describing the desired functionality.

rgov avatar Mar 01 '17 09:03 rgov

I've added serialization of alarms, so they're now written out to the extension's data directory.

The hackiness to get the data directory path is something that should be looked into. I don't really understand the get_core function but I had to make it worse to pipe everything through. I needed access to both (i) the Extension class and (ii) the config. (I tried stealing Mopidy-Local-SQLite's use of from . import Extension but I couldn't get it to work.)

rgov avatar Mar 06 '17 06:03 rgov

This latest commit should address backwards compatibility concerns. Upon upgrading, it will now use the default settings to create the first alarm.

rgov avatar Mar 06 '17 07:03 rgov

@rgov I added you as a collaborator to this repo. :slightly_smiling_face: Feel free to do the needed changes in develop (incl merging this pull request). However I would be happy to do the releasing stuff (incl. pre-release testing) myself.

I guess you are right that develop should not be the stable. Hopefully there won't be many people using it as their daily driver.

P.S. Sorry - I didn't have the time to do the code reviewing yet, but I guess I will do it during release preparation.

DavisNT avatar Mar 06 '17 22:03 DavisNT

@rgov: Hi, will this patch support weekly recurring alarms? For example, sounding the alarm at 10 every Monday and Thursday? Or only multiple alarms a day. Thanks!

fekete-robert avatar May 09 '17 08:05 fekete-robert

It does support weekly recurring alarms, but I don’t think that’s exposed in the web interface yet. You could edit the configuration file, though.

On May 9, 2017, at 1:38 AM, Robert Fekete [email protected] wrote:

@rgov https://github.com/rgov: Hi, will this patch support weekly recurring alarms? For example, sounding the alarm at 10 every Monday and Thursday? Or only multiple alarms a day. Thanks!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/DavisNT/mopidy-alarmclock/pull/8#issuecomment-300099175, or mute the thread https://github.com/notifications/unsubscribe-auth/AAGo39Pw5pbef4YqxcuuxM2vuqwgDf_wks5r4CYIgaJpZM4MMog0.

rgov avatar May 09 '17 16:05 rgov

Sounds great, thanks!

On Tue, May 9, 2017 at 6:38 PM, Ryan Govostes [email protected] wrote:

It does support weekly recurring alarms, but I don’t think that’s exposed in the web interface yet. You could edit the configuration file, though.

On May 9, 2017, at 1:38 AM, Robert Fekete [email protected] wrote:

@rgov https://github.com/rgov: Hi, will this patch support weekly recurring alarms? For example, sounding the alarm at 10 every Monday and Thursday? Or only multiple alarms a day. Thanks!

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub < https://github.com/DavisNT/mopidy-alarmclock/pull/8#issuecomment-300099175>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ AAGo39Pw5pbef4YqxcuuxM2vuqwgDf_wks5r4CYIgaJpZM4MMog0>.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/DavisNT/mopidy-alarmclock/pull/8#issuecomment-300225029, or mute the thread https://github.com/notifications/unsubscribe-auth/ADwKbZfGYoRRlCvcy0EYwejLRtSyhKjbks5r4JadgaJpZM4MMog0 .

fekete-robert avatar May 09 '17 17:05 fekete-robert

@DavisNT What would you like to do with this?

rgov avatar Aug 06 '18 15:08 rgov

@rgov If you are willing to develop the not-yet-done things (e.g. tests) I can add you back to collaborators and do the testing/release part... If not, I will just postpone work on this (as currently there don't seem to be that many users lacking the persistent scheduling of alarms). P.S. When my alarm clock failed on March 2018, I added some more safeguards and probably then I removed you from collaborators (as there were no activity from your side for around a year).

DavisNT avatar Jan 07 '19 23:01 DavisNT

Hi I'm trying to install alarmclock and be able to set multiple alarm the fork og @rgov work well except mopidy is set in repeat mode, I tried to install the develop branch from this repo but there is not multiple alarm support do I need to fork from @rgov to my repository to disable the repeat mode ? thanks

RomanJos avatar Sep 11 '19 16:09 RomanJos

Multiple alarm support is in my fork in the multi-alarm branch. But it has not been updated since February 2017, and since then the mainline of the feature has moved in a different direction. You can try it, but I'm sorry that I cannot provide support as I am working on other projects now.

On Wed, Sep 11, 2019 at 12:51 PM RomanJos [email protected] wrote:

Hi I'm trying to install alarmclock and be able to set multiple alarm the fork og @rgov https://github.com/rgov work well except mopidy is set in repeat mode, I tried to install the develop branch from this repo but there is not multiple alarm support do I need to fork from @rgov https://github.com/rgov to my repository to disable the repeat mode ? thanks

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/DavisNT/mopidy-alarmclock/pull/8?email_source=notifications&email_token=AAA2RX5ZI565WXRY666ZR2TQJEO2ZA5CNFSM4DBSRA2KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD6PEXVQ#issuecomment-530467798, or mute the thread https://github.com/notifications/unsubscribe-auth/AAA2RXYG2UFPYNDMSTB4L53QJEO2ZANCNFSM4DBSRA2A .

rgov avatar Sep 11 '19 17:09 rgov

Thank you for your quick reply, no problem. I will try to modifie it and thank you for your work :smiley:

RomanJos avatar Sep 11 '19 20:09 RomanJos