Add ZoneB support #20
Conversation
WalkthroughThe updates significantly enhance the functionality and control of Zone A/B receivers by introducing new commands and enumerations for audio management. Key additions include variations for "speakersets" and "subzones," with specific API commands for power, volume, and mute features. The changes span multiple files, expanding configuration options and improving the test suite for better coverage of new functionalities. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Server
participant ZoneManager
Client->>Server: Send command for Zone B
Server->>ZoneManager: Process command for Zone B
ZoneManager->>Server: Confirm action (power, volume, mute)
Server->>Client: Send response with updated state
TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Outside diff range, codebase verification and nitpick comments (3)
docs/PRACTICALITIES.md (3)
139-139: Add a comma for clarity.Add a comma after "zone" for better readability.
- These are part of the MAIN zone. On the API these are controlled with these functions + These are part of the MAIN zone. On the API, these are controlled with these functionsTools
LanguageTool
[uncategorized] ~139-~139: Possible missing comma found.
Context: ...hese are part of the MAIN zone. On the API these are controlled with these functio...(AI_HYDRA_LEO_MISSING_COMMA)
151-151: Add a comma for clarity.Add a comma after "subunit" for better readability.
- On the API this subzone is controlled by the following functions. + On the API, this subzone is controlled by the following functions.Tools
LanguageTool
[uncategorized] ~151-~151: Possible missing comma found.
Context: ...re exposed on the MAIN subunit. On the API this subzone is controlled by the follo...(AI_HYDRA_LEO_MISSING_COMMA)
160-160: Add a comma for clarity.Add a comma after "SPEAKERB" for better readability.
- But when controlling the SPEAKERB it will power on/off ZoneB + But when controlling the SPEAKERB, it will power on/off ZoneBTools
LanguageTool
[uncategorized] ~160-~160: Possible missing comma found.
Context: .../B` functions. But when controlling the SPEAKERB it will power on/off ZoneB (assumption ...(AI_HYDRA_LEO_MISSING_COMMA)
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files ignored due to path filters (1)
logs/RX-V583.txtis excluded by!logs/**
Files selected for processing (8)
- docs/PRACTICALITIES.md (1 hunks)
- docs/all_commands_ever_seen.txt (3 hunks)
- tests/test_enums.py (2 hunks)
- tests/test_zone.py (9 hunks)
- ynca/init.py (2 hunks)
- ynca/enums.py (3 hunks)
- ynca/server.py (4 hunks)
- ynca/subunits/zone.py (3 hunks)
Additional context used
LanguageTool
docs/PRACTICALITIES.md
[uncategorized] ~139-~139: Possible missing comma found.
Context: ...hese are part of the MAIN zone. On the API these are controlled with these functio...(AI_HYDRA_LEO_MISSING_COMMA)
[misspelling] ~145-~145: This word is normally spelled as one.
Context: ...SPEAKERB`. e.g. RX-V573 ### Subzone An other variation seen on RX-V583 is a "subzone...(EN_COMPOUNDS_AN_OTHER)
[duplication] ~147-~147: Possible typo: you repeated a word
Context: ...s zone can be powered individually from the the MAIN zone, but wil always have the same...(ENGLISH_WORD_REPEAT_RULE)
[grammar] ~149-~149: Did you mean to use the possessive pronoun “its”?
Context: ...Reason for calling it a subzone is that it's functions are exposed on the MAIN subun...(IT_S_ITS)
[uncategorized] ~151-~151: Possible missing comma found.
Context: ...re exposed on the MAIN subunit. On the API this subzone is controlled by the follo...(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~160-~160: Possible missing comma found.
Context: .../B` functions. But when controlling the SPEAKERB it will power on/off ZoneB (assumption ...(AI_HYDRA_LEO_MISSING_COMMA)
Markdownlint
docs/PRACTICALITIES.md
129-129: null
Bare URL used(MD034, no-bare-urls)
152-152: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
Additional comments not posted (49)
ynca/__init__.py (5)
33-33: LGTM!The addition of
PwrBis consistent and correctly placed.The code changes are approved.
Also applies to: 77-77
38-38: LGTM!The addition of
SpeakerAis consistent and correctly placed.The code changes are approved.
Also applies to: 82-82
39-39: LGTM!The addition of
SpeakerBis consistent and correctly placed.The code changes are approved.
Also applies to: 83-83
43-43: LGTM!The addition of
ZoneBAvailis consistent and correctly placed.The code changes are approved.
Also applies to: 87-87
44-44: LGTM!The addition of
ZoneBMuteis consistent and correctly placed.The code changes are approved.
Also applies to: 88-88
tests/test_enums.py (5)
24-24: LGTM!The addition of
PwrBis consistent and correctly placed.The code changes are approved.
Also applies to: 59-59
29-29: LGTM!The addition of
SpeakerAis consistent and correctly placed.The code changes are approved.
Also applies to: 64-64
30-30: LGTM!The addition of
SpeakerBis consistent and correctly placed.The code changes are approved.
Also applies to: 65-65
34-34: LGTM!The addition of
ZoneBAvailis consistent and correctly placed.The code changes are approved.
Also applies to: 69-69
35-35: LGTM!The addition of
ZoneBMuteis consistent and correctly placed.The code changes are approved.
Also applies to: 70-70
ynca/subunits/zone.py (12)
26-26: LGTM!The addition of
PwrBis consistent and correctly placed.The code changes are approved.
Also applies to: 199-199
29-29: LGTM!The addition of
SpeakerAis consistent and correctly placed.The code changes are approved.
Also applies to: 196-196
30-30: LGTM!The addition of
SpeakerBis consistent and correctly placed.The code changes are approved.
Also applies to: 197-197
34-34: LGTM!The addition of
ZoneBAvailis consistent and correctly placed.The code changes are approved.
Also applies to: 201-201
35-35: LGTM!The addition of
ZoneBMuteis consistent and correctly placed.The code changes are approved.
Also applies to: 202-202
47-55: LGTM!The function
do_vol_upis correctly implemented and improves code modularity.The code changes are approved.
57-65: LGTM!The function
do_vol_downis correctly implemented and improves code modularity.The code changes are approved.
203-203: LGTM!The addition of
zonebnameis consistent and correctly placed.The code changes are approved.
204-209: LGTM!The addition of
zonebvolis consistent and correctly placed.The code changes are approved.
211-213: LGTM!The function
zonebvol_upis correctly implemented and improves code modularity.The code changes are approved.
214-216: LGTM!The function
zonebvol_downis correctly implemented and improves code modularity.The code changes are approved.
184-187: LGTM!The refactoring of
vol_upandvol_downto usedo_vol_upanddo_vol_downimproves code modularity and maintainability.The code changes are approved.
ynca/enums.py (5)
328-340: LGTM!The new enumeration
PwrBis correctly implemented with the_missing_method to handle unknown values.
429-441: LGTM!The new enumeration
SpeakerAis correctly implemented with the_missing_method to handle unknown values.
443-455: LGTM!The new enumeration
SpeakerBis correctly implemented with the_missing_method to handle unknown values.
515-527: LGTM!The new enumeration
ZoneBAvailis correctly implemented with the_missing_method to handle unknown values.
530-541: LGTM!The new enumeration
ZoneBMuteis correctly implemented with the_missing_method to handle unknown values.ynca/server.py (6)
Line range hint
99-121: LGTM!The new entries in the
multiresponse_functions_tableexpand the range of functions that can be processed and are consistent with the existing ones.
227-231: LGTM!The update ensures that the
MEMfunction does not generate an error message, consistent with the behavior described in the comments.
Line range hint
231-241: LGTM!The update allows adjustments to the volume for Zone B independently, enhancing control over the system's audio settings.
273-275: LGTM!The update ensures that the power state of Zone B is correctly managed, consistent with the behavior described in the comments.
284-287: LGTM!The update ensures that the overall system power state reflects the status of individual zones, including Zone B.
Line range hint
99-121: LGTM!The new entries in the
multiresponse_functions_tableexpand the range of functions that can be processed and are consistent with the existing ones.docs/all_commands_ever_seen.txt (6)
227-227: LGTM!The new entry
@MAIN:PWRB=?is consistent with existing entries and follows the same format.The code changes are approved.
244-244: LGTM!The new entry
@MAIN:SPEAKERA=?is consistent with existing entries and follows the same format.The code changes are approved.
245-245: LGTM!The new entry
@MAIN:SPEAKERB=?is consistent with existing entries and follows the same format.The code changes are approved.
257-257: LGTM!The new entry
@MAIN:ZONEBAVAIL=?is consistent with existing entries and follows the same format.The code changes are approved.
258-258: LGTM!The new entry
@MAIN:ZONEBMUTE=?is consistent with existing entries and follows the same format.The code changes are approved.
259-259: LGTM!The new entry
@MAIN:ZONEBVOL=?is consistent with existing entries and follows the same format.The code changes are approved.
tests/test_zone.py (10)
13-13: LGTM!The new constant
PwrBis consistent with existing imports and follows the same format.The code changes are approved.
15-15: LGTM!The new constant
SpeakerAis consistent with existing imports and follows the same format.The code changes are approved.
16-16: LGTM!The new constant
SpeakerBis consistent with existing imports and follows the same format.The code changes are approved.
19-19: LGTM!The new constant
ZoneBMuteis consistent with existing imports and follows the same format.The code changes are approved.
Line range hint
38-55: LGTM!The new entries added to the
INITIALIZE_FULL_RESPONSESlist are consistent with existing entries and follow the same format.The code changes are approved.
87-92: LGTM!The new entry
ZONEBNAMEadded to theINITIALIZE_FULL_RESPONSESlist is consistent with existing entries and follows the same format.The code changes are approved.
189-195: LGTM!The new assertions added to the
test_initialize_fullfunction are consistent with existing assertions and follow the same format.The code changes are approved.
219-231: LGTM!The new test function
test_zoneb_muteis consistent with existing test functions and follows the same format.The code changes are approved.
287-339: LGTM!The new test function
test_zoneb_volumeis consistent with existing test functions and follows the same format.The code changes are approved.
438-448: LGTM!The new test function
test_zonebnameis consistent with existing test functions and follows the same format.The code changes are approved.
|
|
||
|
|
||
| ### Subzone | ||
|
|
There was a problem hiding this comment.
Correct the phrase "An other" to "Another".
The phrase "An other variation" should be "Another variation."
- An other variation seen on RX-V583 is a "subzone" called Zone B.
+ Another variation seen on RX-V583 is a "subzone" called Zone B.Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Another variation seen on RX-V583 is a "subzone" called Zone B. |
Tools
LanguageTool
[misspelling] ~145-~145: This word is normally spelled as one.
Context: ...SPEAKERB`. e.g. RX-V573 ### Subzone An other variation seen on RX-V583 is a "subzone...(EN_COMPOUNDS_AN_OTHER)
docs/PRACTICALITIES.md
Outdated
| An other variation seen on RX-V583 is a "subzone" called Zone B. In the AV Controller app it is shown similar to Zone 2. | ||
| This zone can be powered individually from the the MAIN zone, but wil always have the same input as the MAIN zone. | ||
|
|
||
| Reason for calling it a subzone is that it's functions are exposed on the MAIN subunit. |
There was a problem hiding this comment.
Correct "it's" to "its".
The phrase "it's functions" should use the possessive pronoun "its."
- it's functions are exposed on the MAIN subunit.
+ its functions are exposed on the MAIN subunit.Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Reason for calling it a subzone is that it's functions are exposed on the MAIN subunit. | |
| Reason for calling it a subzone is that its functions are exposed on the MAIN subunit. |
Tools
LanguageTool
[grammar] ~149-~149: Did you mean to use the possessive pronoun “its”?
Context: ...Reason for calling it a subzone is that it's functions are exposed on the MAIN subun...(IT_S_ITS)
docs/PRACTICALITIES.md
Outdated
| ### Subzone | ||
|
|
||
| An other variation seen on RX-V583 is a "subzone" called Zone B. In the AV Controller app it is shown similar to Zone 2. | ||
| This zone can be powered individually from the the MAIN zone, but wil always have the same input as the MAIN zone. |
There was a problem hiding this comment.
Remove the repeated word "the".
The phrase "from the the MAIN zone" contains a repeated word "the."
- from the the MAIN zone
+ from the MAIN zoneCommittable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| This zone can be powered individually from the the MAIN zone, but wil always have the same input as the MAIN zone. | |
| This zone can be powered individually from the MAIN zone, but wil always have the same input as the MAIN zone. |
Tools
LanguageTool
[duplication] ~147-~147: Possible typo: you repeated a word
Context: ...s zone can be powered individually from the the MAIN zone, but wil always have the same...(ENGLISH_WORD_REPEAT_RULE)
There was a problem hiding this comment.
Actionable comments posted: 3
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- docs/PRACTICALITIES.md (1 hunks)
Additional context used
LanguageTool
docs/PRACTICALITIES.md
[uncategorized] ~148-~148: Possible missing article found.
Context: ... have the same input as the MAIN zone. Reason for calling it a subzone is that its fu...(AI_HYDRA_LEO_MISSING_THE)
Markdownlint
docs/PRACTICALITIES.md
129-129: null
Bare URL used(MD034, no-bare-urls)
152-152: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
Additional comments not posted (2)
docs/PRACTICALITIES.md (2)
145-145: Correct the phrase "An other" to "Another".The phrase "An other variation" should be "Another variation."
- An other variation seen on RX-V583 is a "subzone" called Zone B. + Another variation seen on RX-V583 is a "subzone" called Zone B.
149-149: Correct "it's" to "its".The phrase "it's functions" should use the possessive pronoun "its."
- it's functions are exposed on the MAIN subunit. + its functions are exposed on the MAIN subunit.
| Reason for calling it a subzone is that its functions are exposed on the MAIN subunit. | ||
|
|
||
| On the API, this subzone is controlled by the following functions. Note that Mute only supports On/Off | ||
| ``` |
There was a problem hiding this comment.
Specify the language for the fenced code block.
The fenced code block should have a language specified for better readability and syntax highlighting.
@main:PWRB
@main:ZONEBAVAIL
@main:ZONEBNAME # Unknown if this can actually be set on the AVR
@main:ZONEBMUTE
@main:ZONEBVOL
+```plaintext
Tools
Markdownlint
152-152: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
docs/PRACTICALITIES.md
Outdated
| ### Subzone | ||
|
|
||
| Another variation seen on RX-V583 is a "subzone" called Zone B. In the AV Controller app it is shown similar to Zone 2. | ||
| This zone can be powered individually from the MAIN zone, but wil always have the same input as the MAIN zone. |
There was a problem hiding this comment.
Correct the phrase "wil always" to "will always".
The phrase "wil always" should be "will always".
- This zone can be powered individually from the MAIN zone, but wil always have the same input as the MAIN zone.
+ This zone can be powered individually from the MAIN zone, but will always have the same input as the MAIN zone.Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| This zone can be powered individually from the MAIN zone, but wil always have the same input as the MAIN zone. | |
| This zone can be powered individually from the MAIN zone, but will always have the same input as the MAIN zone. |
| @MAIN:ZONEBVOL | ||
| ``` | ||
|
|
||
| Note that this variant also has the `@MAIN:SPEAKERA/B` functions. But when controlling the SPEAKERB it will power on/off ZoneB (assumption is that it works the same for SPEAKERA/MAIN zone PWR). So when implementing a client these should probably be hidden/ignored. |
There was a problem hiding this comment.
Add a missing article.
The phrase "works the same for SPEAKERA/MAIN zone PWR" should include the article "the" before "MAIN zone PWR".
- works the same for SPEAKERA/MAIN zone PWR
+ works the same for SPEAKERA/the MAIN zone PWRCommittable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Note that this variant also has the `@MAIN:SPEAKERA/B` functions. But when controlling the SPEAKERB it will power on/off ZoneB (assumption is that it works the same for SPEAKERA/MAIN zone PWR). So when implementing a client these should probably be hidden/ignored. | |
| Note that this variant also has the `@MAIN:SPEAKERA/B` functions. But when controlling the SPEAKERB it will power on/off ZoneB (assumption is that it works the same for SPEAKERA/the MAIN zone PWR). So when implementing a client these should probably be hidden/ignored. |
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- docs/PRACTICALITIES.md (1 hunks)
Additional context used
Markdownlint
docs/PRACTICALITIES.md
129-129: null
Bare URL used(MD034, no-bare-urls)
152-152: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
Additional comments not posted (6)
docs/PRACTICALITIES.md (6)
145-145: Correct the phrase "An other" to "Another".The phrase "An other variation" should be "Another variation."
- An other variation seen on RX-V583 is a "subzone" called Zone B. + Another variation seen on RX-V583 is a "subzone" called Zone B.
147-147: Remove the repeated word "the".The phrase "from the the MAIN zone" contains a repeated word "the."
- from the the MAIN zone + from the MAIN zone
147-147: Correct the phrase "wil always" to "will always".The phrase "wil always" should be "will always".
- This zone can be powered individually from the MAIN zone, but wil always have the same input as the MAIN zone. + This zone can be powered individually from the MAIN zone, but will always have the same input as the MAIN zone.
149-149: Correct "it's" to "its".The phrase "it's functions" should use the possessive pronoun "its."
- it's functions are exposed on the MAIN subunit. + its functions are exposed on the MAIN subunit.
152-152: Specify the language for the fenced code block.The fenced code block should have a language specified for better readability and syntax highlighting.
@main:PWRB
@main:ZONEBAVAIL
@main:ZONEBNAME # Unknown if this can actually be set on the AVR
@main:ZONEBMUTE
@main:ZONEBVOL+```plaintextTools
Markdownlint
152-152: null
Fenced code blocks should have a language specified(MD040, fenced-code-language)
160-160: Add a missing article.The phrase "works the same for SPEAKERA/MAIN zone PWR" should include the article "the" before "MAIN zone PWR".
- works the same for SPEAKERA/MAIN zone PWR + works the same for SPEAKERA/the MAIN zone PWR
Summary by CodeRabbit
New Features
Bug Fixes
Tests