jkossis commented 2 years ago

Adding Height Audio As A Modifiable EQ Option

Description

Add the HeightChannelLevel EQ type, allowing for the height audio to be retrieved and set.

I was able to confirm that HeightChannelLevel is a valid EQ type using Wireshark (along with my Arc). When I would modify the height audio using the official iOS app, I saw the following POSTed to /MediaRenderer/RenderingControl/Control

<s:Envelope
    xmlns:s="http://schemas.xmlsoap.org/soap/envelope/"
        s:encodingStyle="http://schemas.xmlsoap.org/soap/encoding/">
    <s:Body>
        <u:SetEQ
            xmlns:u="urn:schemas-upnp-org:service:RenderingControl:1">
            <InstanceID>
                    0
                    </InstanceID>
            <EQType>
                    HeightChannelLevel
                    </EQType>
            <DesiredValue>
                    4
                    </DesiredValue>
        </u:SetEQ>
    </s:Body>
</s:Envelope>

Your checklist for this pull request

🚨 Please review the guidelines for contributing to this repository.

[x] Make sure you are requesting to pull a topic/feature/bugfix branch (right side). Don't request your master!
[x] Make sure you are making a pull request against the beta branch (left side). Also you should start your branch off svrooij/node-sonos-ts/beta.
[x] Check the commit's or even all commits' message styles matches our requested structure.
[x] Check your code additions will fail neither code linting checks nor unit test.
[x] Check you add tests for added code.

💔 Thank you!

coveralls commented 2 years ago

Pull Request Test Coverage Report for Build 3629843274

0 of 1 (0.0%) changed or added relevant line in 1 file are covered.
No unchanged relevant lines lost coverage.
Overall coverage decreased (-0.02%) to 81.563%

Changes Missing Coverage	Covered Lines	Changed/Added Lines	%
src/services/device-properties.service.ts	0	1	0.0%
<!--	Total:	0	1	0.0%	-->

Totals
Change from base Build 2942464451:	-0.02%
Covered Lines:	1684
Relevant Lines:	2020

💛 - Coveralls

svrooij commented 1 year ago

These files are generated from the service discovery combined with the manual documentation file. This is a great addition, but it will get lost upon the next re-generation.

Can I suggest you to submit a pr for https://github.com/svrooij/sonos-api-docs/blob/c6829c551717cd1c34f5842bab340fcf46edd883/docs/documentation.json#L360 Then

It gets in this library (proces not yet automated)
Your discovery is also visible to others reading just the documentation.
Your discovery would also appear in my latest c# version of the Sonos library.

jkossis commented 1 year ago

@svrooij I put up there docs pr here.

Looks like it failed linting, but for something unrelated to what I added, just an fyi.

svrooij commented 1 year ago

It's merged! Want to do the honors to regenerate this library and that way add your name to the changes?

jkossis commented 1 year ago

@svrooij just did the re-generation, thank you!

One thing I noted was, it wiped away rendering control service's HeightChannelLevel that I added in this pr, which makes sense. It doesn't come back in the discovery doc, though it does actually get returned in the real world (I tested this).

Along these lines, is there a mechanism in place to supplement events properties that don't come back in the discovery doc?

svrooij commented 1 year ago

@jkossis could you execute the following commands on your branch?

npm install -g @svrooij/sonos-docs
# and then
npm run generate

this should update the generator and allows for the added variable in the RenderingControlService

jkossis commented 1 year ago

@svrooij Absolutely! Will do that first thing in the morning (EST).

jkossis commented 1 year ago

@svrooij just made the change!

github-actions[bot] commented 1 year ago

:tada: This PR is included in version 2.6.0-beta.2 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

svrooij / node-sonos-ts