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 methods to retrieve and change verify_ssl_switch #39

Open
wants to merge 1 commit into
base: feature/no_verify_ssl
Choose a base branch
from

Conversation

lukshan13
Copy link

@lukshan13 lukshan13 commented Mar 21, 2024

To facilitate dynamic control of SSL Verify, once an integration has already been added into home assistant, added new methods in the following files:

omadaapiconnecttion.py

get_verify_ssl_switch() -> Bool : retrieves current state of 'verify_ssl' class property
set_verify_ssl_switch(bool) -> None : sets 'verify_ssl' class property

omadaclient.py

get_verify_ssl_switch() -> Bool : Wrapper function for apiconnection
set_verify_ssl_switch(Bool) -> None : Wrapper function for apiconnection

Next steps: Once merged, add method in HomeAssistant core Omada integration to show and modify the state from integration settings

references issue #40

@lukshan13
Copy link
Author

Btw, i made a pull request into the branch that I thought was appropriate. LMK if i need to merge this into master on my end first, then make a pull request to your repo

@MarkGodwin
Copy link
Owner

I think this will probably work, but I'm not sure it's necessary. It's unlikely you'd need to change the verify_ssl flag dynamically. I would have thought you'd want to change the connection address, port number, etc. more often than the verify flag, and the API doesn't allow that either.
However... I think if the flag value needs to be changed, you would just dispose of the API object and create whole a new one with the updated connection values. I don't see there is a need to keep the same OmadaClient if you want to change the verify_ssl flag.

But, if you really want this change in, I guess I don't have a problem with merging it.

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.

2 participants