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

Add Room Media API Calls #21

Merged
merged 5 commits into from
Feb 15, 2023
Merged

Add Room Media API Calls #21

merged 5 commits into from
Feb 15, 2023

Conversation

nalin29
Copy link
Contributor

@nalin29 nalin29 commented Feb 11, 2023

Second Pull Request with Room API Calls authored by @nalin29

pyControl4/room.py Outdated Show resolved Hide resolved
{},
)

async def getAudioDevices(self):
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you document/provide an example of what kind of data format is returned by this call?

See here for an example:

async def getAccountControllers(self):

Copy link
Contributor Author

@nalin29 nalin29 Feb 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The API call on OS 3.2.3, just returns None for me on both getAudioDevices/getVideoDevices. I do not make use of the functions in the HA integration instead we use the UI. Perhaps we delete these API Calls since they are not used?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deleted.

Copy link
Owner

@lawtancool lawtancool Feb 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It returns None? Like literal Python None or some sort of valid JSON response that just says no devices?

I'm inclined to leave it in with a comment that says it returns none on OS 3.2.3 if it's actually a valid JSON response from Control4. Perhaps other configurations/older OS versions make use of this call.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah its just None the python literal. That is why I labeled as an invalid API call and deleted but I can put it back with comment that it wasn't valid in OS 3.2.3

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done, I committed a change that brings back the functions and adds a warning note.

pyControl4/room.py Outdated Show resolved Hide resolved
pyControl4/director.py Outdated Show resolved Hide resolved
@lawtancool
Copy link
Owner

After we figure out that comment on getAudioDevices/getVideoDevices we should be good to merge!

@nalin29
Copy link
Contributor Author

nalin29 commented Feb 15, 2023

Should I rebase and squash all commits together, or is this fine

@lawtancool
Copy link
Owner

Should I rebase and squash all commits together, or is this fine

This is fine, it'll all be squashed by GitHub on merge

@lawtancool lawtancool merged commit 1ce1035 into lawtancool:master Feb 15, 2023
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