Skip to content
New issue

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

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

Already on GitHub? # to your account

Basic intercom support #277

Closed

Conversation

rautsch
Copy link

@rautsch rautsch commented Dec 4, 2022

Added basic support for the Ring Intercom.
The main focus was on the ability to trigger door opening and send invitations.

@tispokes
Copy link

tispokes commented May 5, 2023

Would be nice to have this feature

@lastfreeusernameicouldfind
Copy link

lastfreeusernameicouldfind commented Aug 19, 2023

why is this PR hanging since Dec 2022? Would be very nice to get this feature merged.
Please can this be merged OR else please let me know whats the reason to not merge it

@sdb9696
Copy link
Member

sdb9696 commented Nov 27, 2023

HI @rautsch, I have recently been added as a maintainer on this library. There seems to be some demand on homeassistant for this feature. Would you be willing to bring this PR up to date so I can review it and we can test it with your intercom devices?

@sdb9696 sdb9696 marked this pull request as draft November 27, 2023 10:26
@rautsch
Copy link
Author

rautsch commented Nov 27, 2023

HI @rautsch, I have recently been added as a maintainer on this library. There seems to be some demand on homeassistant for this feature. Would you be willing to bring this PR up to date so I can review it and we can test it with your intercom devices?

Hey,
Great to hear! I think I can look into it in the next few days.
What needs to be done in order to merge? (I guess tests, docu, extend feature support)

@sdb9696
Copy link
Member

sdb9696 commented Nov 27, 2023

Hi @rautsch. Yes, basically it looks pretty good but you'll need to rebase to the latest code. N.B. the library has a cli now which might help you with integration testing. So:

  1. Rebase - I'd recommend squashing all your commits down to one commit first so the rebase is easier.
  2. Docs - (don't worry about too much detail here, we can always improve later)
  3. Add tests and mock fixtures for the new api calls. This PR shows what's needed and should help with it.
  4. Rename device_id in the constructor to device_api_id to avoid confusion with the device_id value returned from the api (which is usually the mac)

Thanks!

@andrew-rinato
Copy link
Contributor

I am actually just working on this myself. I hope to have something up today for review. Added a couple more settings setters as well.

@sdb9696
Copy link
Member

sdb9696 commented Nov 29, 2023

Hi @andrew-rinato. Are the additional settings setters needed for the intercom devices or are they independent? If they're independent it would be better to have a separate PR to ease the review.

@andrew-rinato
Copy link
Contributor

Hi @sdb9696 I just created a separate PR. You'll see the device settings I've added, which are specific to the intercom "other" device category. Most correspond to settings in the app, but there is an additional one I exposed that I believe will be of interest to many others (gauging from dgreif/ring#1083).

@sdb9696
Copy link
Member

sdb9696 commented Feb 5, 2024

Closing as addressed by #330

@sdb9696 sdb9696 closed this Feb 5, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Apr 18, 2024
# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants