Closed mattijsbliek closed 2 years ago
I'll have to check on the parallel reset commands. I recall doing them with a delay between them in order to make sure they were processed correctly by the controller. Maybe a bug on the sonos side?
On Mon, Aug 9, 2021 at 4:23 AM Mattijs Bliek @.***> wrote:
Finally an actual feature 😅 This PR makes it configurable whether to reset playback options when new music is queued.
It's build on top of the Add logger https://github.com/ryanolf/node-sonos-nfc/pull/6 PR, so my suggestion would be to review that one first.
Important bits
- Extract the resetting of playback options to a separate function, so it can be independently tested and the process_sonos_command becomes easier to follow.
- Fire reset commands are in parallel via Promise.all. I don't know if this actually works since I don't yet have a hardware setup to test with.
- Keep the responsibility of which command should reset playback options in process_sonos_command.
- Extract get_sonos_url so it can be used everywhere.
You can view, comment on, or merge this pull request online at:
https://github.com/ryanolf/node-sonos-nfc/pull/7 Commit Summary
- Add logger.
- Fix config class not working in tests.
- Add supported log levels to README.
- Make playback options reset configurable.
- Remove redundant function argument.
- Strict checking of reset_playback_options_on_queue config option.
File Changes
- M README.md https://github.com/ryanolf/node-sonos-nfc/pull/7/files#diff-b335630551682c19a781afebcf4d07bf978fb1f8ac04c6bf87428ed5106870f5 (52)
- A lib/mocks/logger.js https://github.com/ryanolf/node-sonos-nfc/pull/7/files#diff-fc112f871a1bc9cad770e8e6e8ab8aa278e5549f835c429fc1ef5b7d39972c30 (9)
- A lib/tests/config.test.js https://github.com/ryanolf/node-sonos-nfc/pull/7/files#diff-5cfeed071f7411c489e271ff46eee2cb21552938a33b6a1f224d226e96621d14 (35)
- A lib/tests/get_sonos_url.test.js https://github.com/ryanolf/node-sonos-nfc/pull/7/files#diff-45cdcea2c0a60503d27e37c76024d32fda2b8bffefbdcf6468788fdf50af0750 (24)
- M lib/tests/process_sonos_command.test.js https://github.com/ryanolf/node-sonos-nfc/pull/7/files#diff-84a2cb27f704a1a3b86bc6c1d750a4e61fba9000339b66ea7eabb94502e2ae0e (56)
- A lib/tests/reset_playback_options.test.js https://github.com/ryanolf/node-sonos-nfc/pull/7/files#diff-3076027fe2278f3b33ab38dc2aada9eb7d164b850b166fb4d8f8b16850e81199 (41)
- M lib/tests/sonos_nfc.test.js https://github.com/ryanolf/node-sonos-nfc/pull/7/files#diff-29d1cb91c4debd71858270dffe4531d30da0dde396043485ab8d97631a9d6957 (26)
- A lib/config.js https://github.com/ryanolf/node-sonos-nfc/pull/7/files#diff-d5109920b34c2b2e141bd41232d8876432a17479c388e142d8a036bb7b36ea62 (48)
- A lib/get_sonos_url.js https://github.com/ryanolf/node-sonos-nfc/pull/7/files#diff-52ece70f590d68adc98bf7cbcde30c15373f0393c9a8c3d744cfc55e21f26627 (13)
- A lib/logger.js https://github.com/ryanolf/node-sonos-nfc/pull/7/files#diff-9c6fae252a0db7a8d112dee362cbc6b40443ab013ad80bc29b233406cf6fd546 (34)
- M lib/process_sonos_command.js https://github.com/ryanolf/node-sonos-nfc/pull/7/files#diff-9915b77b60e8d0d816bf0291f0196729a19538e11e89a4c7575405cf9910ac07 (63)
- A lib/reset_playback_options.js https://github.com/ryanolf/node-sonos-nfc/pull/7/files#diff-b1d604ab374a3c63e18ce030526f8279165f67cf7432e53fc89e25cc285c992c (28)
- M lib/sonos_nfc.js https://github.com/ryanolf/node-sonos-nfc/pull/7/files#diff-868a04be6e5818a04ea2c079797691391d585d8fb2b708615d6c988642175be4 (21)
- M package-lock.json https://github.com/ryanolf/node-sonos-nfc/pull/7/files#diff-053150b640a7ce75eff69d1a22cae7f0f94ad64ce9a855db544dda0929316519 (8532)
- M package.json https://github.com/ryanolf/node-sonos-nfc/pull/7/files#diff-7ae45ad102eab3b6d7e7896acd08c427a9b25b346470d7bc6507b6481575d519 (4)
- M usersettings.json https://github.com/ryanolf/node-sonos-nfc/pull/7/files#diff-8957e9fecd8ad7866f1b3e83a43db5ca06b5236182e93c4d27032006b82c0a31 (4)
Patch Links:
- https://github.com/ryanolf/node-sonos-nfc/pull/7.patch
- https://github.com/ryanolf/node-sonos-nfc/pull/7.diff
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/ryanolf/node-sonos-nfc/pull/7, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJSBSQFUZHLP6PEI2LZITDT363DJANCNFSM5BZZ6GFA . 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&utm_campaign=notification-email .
Finally an actual feature 😅 This PR makes it configurable whether to reset playback options when new music is queued.
It's build on top of the Add logger PR, so my suggestion would be to review that one first.
Important bits
process_sonos_command
becomes easier to follow.Promise.all
. I don't know if this actually works since I don't yet have a hardware setup to test with.process_sonos_command
.get_sonos_url
so it can be used everywhere.