Skip to content

Added group endpoints#161

Closed
pahenning wants to merge 9 commits intodeborahgu:mainfrom
pahenning:group_endpoints
Closed

Added group endpoints#161
pahenning wants to merge 9 commits intodeborahgu:mainfrom
pahenning:group_endpoints

Conversation

@pahenning
Copy link
Contributor

@pahenning pahenning commented Feb 10, 2026

EDITED:

The workflow issue is clarified, see #184

2.Several additions have been made to implement group endpoints in /marge. This functionality is available for ST10 devices that may be grouped as stereo pairs.

a.) GET /marge/streaming/account/{account}/device/{device}/group
==> for existing ST10 devices, returns if ungrouped, otherwise the full group info file as stated in #131

b.) POST /marge/streaming/account/{account}/group
==> with a group info file as payload creates this group and returns the group info file with group id. These group ids are currently created as 7-digit random number. Several checks are made for the correctness of the payload file. Locally, in the soundcrk datastore, the group info file is currently put into the {account}/devices directory.

c.) POST /marge/streaming/account/{account}/group/{group}
==> with an XML file as payload will modify the name of an existing group

d.) DELETE /marge/streaming/account/{account}/group/{group}
==> will delete an existing group

e.) Some service endpoints for easier handling have been added

    #---------------- listgroups ----------------------------------------
    # list all groups and their devices
    # call as GET /service/account/{account}/listgroups
    # returns <groups>...</groups>
 
   #---------------- creategroup ----------------------------------------
    # create a stereo pair (=group) 
    # call as GET /service/account/{account}/creategroup?master={device1}&slave={device2}
    # returns GROUP_OK or GROUP_ERROR

    #---------------- modgroup ----------------------------------------
    # rename a stereo pair (=group) 
    # call as GET /service/account/{account}/modgroup?groupid={groupid}&newname={newname}
    #   or as GET /service/account/{account}/modgroup?name={name}&newname={newname}
    # returns GROUP_OK or GROUP_ERROR

    #---------------- removegroup ----------------------------------------
    # remove a stereo pair (=group) 
    # call as GET /service/account/{account}/removegroup?groupid={groupid} 
    #   or as GET /service/account/{account}/removegroup?name={name} 
    # returns GROUP_OK or GROUP_ERROR

@akpdw
Copy link
Collaborator

akpdw commented Feb 17, 2026

@pahenning #160 has been merged so that part of the PR can be removed.

I have access to two ST10's this week so I'll try to test out the functionality.

@pahenning pahenning changed the title Added group endpoints, allow for empty containerArt Added group endpoints Feb 17, 2026
@pahenning pahenning closed this Feb 17, 2026
@pahenning pahenning reopened this Feb 17, 2026
@gmuth
Copy link
Contributor

gmuth commented Feb 18, 2026

The existing Bose APIs are sufficient to support stereo pairing. There is no need for any extra endpoints. Only requests from device clients need to be implemented in soundcork. We probably won’t be able to change the behavior of the device implemented by the firmware. All user interaction should be directed to the device only. That’s how the distributed system was build by Bose.

@pahenning
Copy link
Contributor Author

pahenning commented Feb 18, 2026

The existing Bose APIs are sufficient to supply stereo pairing. There is no need for any extra endpoints.

I strongly disagree.

You have claimed that the ST10 requests an answer from the marge...group on startup. This is not true for the hardware that I run.

You have claimed that an addGroup request from anywhere to one of the ST10 boxes is sufficient to set up the group, which implies that the single box 1.contacts the marge endpoint to acquire a group id and 2. also pushes this to the second box. This is not true for the hardware that I run.

Consequently either an external application would be needed that handles the pairing workflow, or it is implemented in the soundcork server. The second approach is preferable in any sense, in particular as this avoids handling XML data outside of Box + soundcork.

Regards
pah

@gmuth
Copy link
Contributor

gmuth commented Feb 18, 2026

@pahenning, Why should I waste time and make up and document non existing behavior? I am neither an AI nor do I hallucinate device-to-marge-api calls. Thank you for your effort to implement the API I documented! I've pulled your branch (about a week ago), fixed a few minor issues (like the trailing / issue I told you before) and all is fine - you have really done a great job! (I had planned to create a PR including some fixes.)

@pahenning
Copy link
Contributor Author

Fine, my congratulations, apparently you have magical hardware which I do not have. If the marge endpoints are enough for you I suggest that you refrain from further comments. I will certainly leave them in the code.

Regards
pah

@akpdw
Copy link
Collaborator

akpdw commented Feb 27, 2026

@pahenning Hey since this works (even if there's some disagreement on how much of it needs to work :) )) we'd like to get this merged in. But we'd like to have some changes made to conform to the design of the rest of the code base. If you have time, could you:

  • Resolve conflicts
  • Run the linter and formatter for the repository as described in https://github.com/deborahgu/soundcork/wiki/Development:-code-style
  • Add a class for Group and switch the code in datastore.py to use the group xml just for serialization and deserialization, and do business logic using the Group object rather than parsing the xml every time.
  • Separate out the marge-replacement endpoints and the service endpoints (and related method) in groups.py into two separate files. (love that you separated it out in the first place; we really need to do the same for a lot of endpoints in main.py)
  • We use urllib.request for http requests everywhere else, so switch to it instead of importing httpx.
  • Use HTTPStatus.* constants instead of numbers for HTTP status codes (yeah there's a lot of our older code that just uses numbers that we haven't cleaned up yet, but we're trying to enforce that for new code)

Oooh after I write that out it sounds like a lot. If you don't have time I understand.

Thanks so much for the contribution (both here and in the testing/issue reporting)!

@pahenning
Copy link
Contributor Author

pahenning commented Feb 27, 2026 via email

@akpdw
Copy link
Collaborator

akpdw commented Mar 8, 2026

@pahenning Made the changes and merged as PR#222. As I don't see the same behavior as you I was not able to test the service endpoints, but I tried to change them as little as possible so hopefully they still work.

Thanks for the PR, and I hope that your vacation is/was great!

@akpdw akpdw closed this Mar 8, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants