search

ryanolf / node-sonos-nfc

Access your streaming music through physical NFC tokens, like putting on a CD or record. Great for children and adult aesthetics.
MIT License
36 stars 14 forks source link

Implementing user definable behaviour for repeat, shuffle, and crossfade #12

Closed ashenshugarRET closed 2 years ago

ashenshugarRET commented 2 years ago

I've split the previous commit, this only covers the changes to make the repeat, shuffle and crossfade reset behaviour definable in usersettings.json. If these changes pass the testing, I'll submit a pull request for the changes to TuneIn command coding and the addition of a "Favorites" command.

ashenshugarRET commented 2 years ago

Hello,

Is there any chance you could try and merge this pull request?

Thanks

ryanolf commented 2 years ago

Hi, thanks for your patience. I’ve been pretty busy— I’ll try to look tonight (pacific time).

On Tue, Mar 15, 2022 at 4:33 AM ashenshugarRET @.***> wrote:

Hello,

Is there any chance you could try and merge this pull request?

Thanks

— Reply to this email directly, view it on GitHub https://github.com/ryanolf/node-sonos-nfc/pull/12#issuecomment-1067884971, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJSBSV2Z4QHHCLJYID2PI3VABYPJANCNFSM5MWECGAA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you are subscribed to this thread.Message ID: @.***>

ashenshugarRET commented 2 years ago

Thanks, I understand.

ryanolf commented 2 years ago

Hi @ashenshugarRET, just suggested a few tweaks in the code. The main thing is that for consistency with the URIs written to the cards for Apple, etc, we should use ":" as a delimiter. I'm happy for you to add "/" as an acceptable option as well if you'd like. I like being permissive.

The other thing is that the tests were not passing on my machine. The CI is still pending I think. Please run the test suite before you update the PR. Thanks!

ashenshugarRET commented 2 years ago

Apologises, I'm not super experienced with Github etc, to be honest, I don't know what the CI is or in fact how to run it. or the test suite.

ryanolf commented 2 years ago

You can run the test suite on your local repo with npm test. I just pulled the updates and tests are still failing. I think I can fix if you give me a minute.

ashenshugarRET commented 2 years ago

Thanks, yours is the first repository I've used that has test etc, so a bit of a steep learning curve. Thanks for your help resolving the problems.

ryanolf commented 2 years ago

Ok, we got it. Thanks for contributing.

ashenshugarRET commented 2 years ago

Thanks, the node-sonos-http-api has just added the BBC Sounds service to stream BBC radio stations which I was hoping to add to this repository to as I think it will be very popular. Would it be OK if I generate another pull request to implement this too?

ryanolf commented 2 years ago

Yeah, sure! Please add tests to the test suite for it and run the test suite before you file the pull requests. You run the tests on your local machine with npm test in the root folder of the project. You can read more about running tests with npm in their docs.

Ryan

On Wed, Mar 16, 2022 at 11:58 AM ashenshugarRET @.***> wrote:

Thanks, the node-sonos-http-api has just added the BBC Sounds service to stream BBC radio stations which I was hoping to add to this repository to as I think it will be very popular. Would it be OK if I generate another pull request to implement this too?

— Reply to this email directly, view it on GitHub https://github.com/ryanolf/node-sonos-nfc/pull/12#issuecomment-1069485786, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJSBSXP6VKFXKOHNIYQT6LVAIVL3ANCNFSM5MWECGAA . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you modified the open/close state.Message ID: @.***>