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

Switch to node-doorbird library #107

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ihrigb
Copy link

@ihrigb ihrigb commented Dec 20, 2023

Resolves #100

I did a first draft on how the library could be integrated. This is not yet tested.

@mcm1957
Copy link
Member

mcm1957 commented Dec 20, 2023

Please respond if you did test the change.

Why did xou remove existing awaits? Except when setting states its almost aleays required tobwait for the async code to complete

@Schmakus
Copy link
Collaborator

I will check the changes bevor merging.

@mcm1957 mcm1957 requested a review from Schmakus December 20, 2023 12:53
@mcm1957
Copy link
Member

mcm1957 commented Dec 20, 2023

I will check the changes bevor merging.

Thanks

Please take care of clean shutdown (library does not keep any open timers as this might break compact mode) and handling of response timemouts

And please check the benefits of an external library compared to existing code too. What oeoblems will ve fixed vy the change?

External might reduce maintainance effort

But problems at external code can be fixed by external dev only.

And this adapter us secztity relevant. Passing username and password to an external libray could be a security risk. I suggest to discuss with @Apollon77 especialky if the extetbal library us not widely used.

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.

Suggestion to use Doorbird client library
3 participants