Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The update_volume method should be removed ? #79

Open
SantiagoSotoC opened this issue Oct 17, 2024 · 3 comments
Open

The update_volume method should be removed ? #79

SantiagoSotoC opened this issue Oct 17, 2024 · 3 comments

Comments

@SantiagoSotoC
Copy link
Contributor

The update_volume method in the client class does not make sense, in my opinion it should be removed, there is the set_volume method that performs the function correctly.

In my opinion it just makes the use of the library confusing.

@luar123
Copy link
Contributor

luar123 commented Oct 17, 2024

Yes it is confusing. Mostly because of missing documentation.
But there is a fundamental difference:
update_volume changes the internal state of the client object only. It is called by server.py when another control client changes the volume and therefore a notificatoon from the server is received. It should never be called by the user as it would lead to a disconnect between the server and the client object.
set_volume sends the request to the server and changes the internal state. So this should.be called by the user.

Same for other update_ and set_ methods.

@SantiagoSotoC
Copy link
Contributor Author

I had not seen it in the server class, I searched with the ide and did not find anything.

So it should be indicated in the docstring ?

@luar123
Copy link
Contributor

luar123 commented Oct 17, 2024

self._clients.get(data.get('id')).update_volume(data)

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

No branches or pull requests

2 participants