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 FCMv1 for push messages #1435

Merged
merged 12 commits into from
Jun 21, 2024
Merged

Switch to FCMv1 for push messages #1435

merged 12 commits into from
Jun 21, 2024

Conversation

tsightler
Copy link
Collaborator

@tsightler tsightler commented Jun 20, 2024

This is a PR primarily for discussion purposes, it is not at this point ready to merge or even close. At the moment it doesn't even work, but it's at least the framework for what could work. The biggest issue right now is the inability to parse the message. Previously there was a single property, gcmData, which contained JSON as a string, but now there are multiple components to an FCM message, and they each contain a string, so effectively it is a JSON object with multiple properties each containing a JSON as a string. We'd need to parse each property to build back the full JSON object. Below is an example of a simple motion push message (some values changed for privacy reasons):

{
  message: {
    data: {
      img: '{"snapshot_uuid":"099a5f65-2132-7255-4633-8ac6abcf83f54:49661945"}',
      analytics: '{"server_correlation_id":"d8ca3f00-0ced-19c8-84e7-b0d77ace2d0a","server_id":"com.ring.pns","subcategory":"human","triggered_at":1718856233917,"sent_at":1718856234001,"referring_item_type":"device_id","referring_item_id":"49661945"}',
      android_config: '{"category":"com.ring.pn.live-event.motion","body":"There is a Person at your Front Porch"}',
      version: '2.0.0',
      data: '{"device":{"e2ee_enabled":false,"id":51762341,"kind":"stickup_cam_elite","name":"Front Porch"},"event":{"ding":{"id":"7382436789123417913","created_at":"2024-06-20T04:03:53Z","subtype":"human","detection_type":"human"},"eventito":{"type":"human","timestamp":1718856233917},"riid":"ea593214c63cb46516bd2157e7a65cf8","is_sidewalk":false,"live_session":{"active_streaming_profile":"rms","default_audio_route":"loud_speaker","max_duration":600}},"location":{"id":"a9ce28cb-fafb-421c-7935-91212131cbd3"}}'
    },
    from: '876313859327',
    priority: 'normal',
    fcmMessageId: 'e8080831-8e8a-4e69-a3f7-72eb00eb7f06'
  },
  persistentId: '0:1718856234110036%7031b2e6f9fd7ecd'
}

Part of me thought that perhaps we should just parse and internally build a notification that matched the old stuff, but I decided instead to attempt to modify the other consumers since there were only a few (camera and intercom) and it probably makes sense to try to use the new format as it seems Ring is trying to standardize the format overall, which makes sense.

I completely removed alarm and battery level handling for now as I did not have examples of those and I couldn't really see it being used anywhere in the code anyway. Certainly once we get things working we can add it back.

I'll work to fix the parsing, at first I didn't understand why it was being parsed from to string and back to JSON, but now I understand why since the properties hold actual JSON as strings. So weird.

This is the initial commit for work to switch to FCMv1 push messages due to imminent deprecation of old method.
@tsightler tsightler marked this pull request as draft June 20, 2024 04:18
@tsightler tsightler requested a review from dgreif June 20, 2024 04:18
@tsightler
Copy link
Collaborator Author

tsightler commented Jun 20, 2024

I've pushed a new update that at least seems to work, push messages are received, events are delivered to devices, etc. There's still work to do but, with only a few minor changes to due to the new push notification format, ring-mqtt is working with this version. Any consumer that wasn't parsing out the push data directly should still work, but I'll need to do more testing to confirm this.

There's also still an issue with upgrading, if you have the legacy credentials, it successfully logs in, but can't decrypt any of the received messages. Probably we need to detect if you have legacy credentials and not attempt to use them as old credentials had pnc.fcm.pushSet, while new credentials have pnc.fcm,installation. Also, also, credentials now include "config" property as well, so seems like it should be fairly trivial to get this working. I'll try to get it done tomorrow.

@tsightler
Copy link
Collaborator Author

Pushed a change to automatically detect legacy push credentials and re-register using the new ones on upgrade. This is now automatic and seem to work.

I published a 12.2.0-alpha version of the package scoped to my npm account so that I could publish a dev version of ring-mqtt that consumes it. Upgrading to the dev version worked perfectly, new credentials were acquired and new FCM token was pushed to Ring, and Ring even accepted this token without me having to clear the account from Control Center. I'm not sure if that will work 100% of the time, but it worked for my dev instance, although it took a few minutes for push notifications to start showing up.

I think we might be close to being able to merge this (it lints and builds now at least). I have not tested the homebridge-ring changes (there was only a small change), and I'm also not 100% sure about the changes I made to the logic for adding and removing dings by Id, although it appears to be working.

Probably the biggest question mark is intercom, I have no idea if I have the correct settings for that since I don't have an intercom. My thought is I will try to recruit someone from the ring-mqtt community to run the dev branch and send me the debug output (I'm currently logging all push notifications).

@tsightler tsightler marked this pull request as ready for review June 20, 2024 23:05
@dgreif
Copy link
Owner

dgreif commented Jun 21, 2024

I opened Eneris/push-receiver#21 to remove debug messaging from push-receiver. Not sure if we want to wait on that to merge or not before we move forward 🤔. If we go with a beta release, I think the extra logging is probably fine for now

@dgreif dgreif changed the base branch from main to beta June 21, 2024 18:35
@dgreif dgreif merged commit 614621c into beta Jun 21, 2024
4 checks passed
@dgreif dgreif deleted the tsightler-fcmv1 branch June 21, 2024 18:52
dgreif added a commit that referenced this pull request Aug 4, 2024
* Switch to FCMv1 for push messages

This is the initial commit for work to switch to FCMv1 push messages due to imminent deprecation of old method.

* Parse each message data property

* Revert accidental werift version change

* Fix lint failures

* Force new FCM registration on legacy credentials

* Update tsconfig.json

* Update types for notifications

* Add changelog

* Handle lack of img field on notifications

* Send all notifications to related device ids

* Log unknown push notifications

---------

Co-authored-by: Tom Sightler <tom.sightler@veeam.com>
Co-authored-by: dgreif <dustin.greif@gmail.com>
koush pushed a commit to koush/ring that referenced this pull request Sep 26, 2024
* Switch to FCMv1 for push messages

This is the initial commit for work to switch to FCMv1 push messages due to imminent deprecation of old method.

* Parse each message data property

* Revert accidental werift version change

* Fix lint failures

* Force new FCM registration on legacy credentials

* Update tsconfig.json

* Update types for notifications

* Add changelog

* Handle lack of img field on notifications

* Send all notifications to related device ids

* Log unknown push notifications

---------

Co-authored-by: Tom Sightler <tom.sightler@veeam.com>
Co-authored-by: dgreif <dustin.greif@gmail.com>
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