Paths

Table of Contentst

[native] Send PONG when PING is received
ClosedPublic
Actions

Authored by tomek on Feb 8 2022, 2:58 PM.

Details

Reviewers

• karol
ashoat
atul
• jim

Commits

rCOMM1e26f7e47044: [native] Send PONG when PING is received

Summary

Check background notification type and react accordingly

Depends on D3133

Test Plan

Send a background notification on Android and check if the code on C++ was executed

{
  data: {
    backgroundNotifType: 'PING',
  },
}

Diff Detail

Repository

rCOMM Comm

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

tomek created this revision.Feb 8 2022, 2:58 PM

Herald added subscribers: atul, • adrian, ashoat. · View Herald TranscriptFeb 8 2022, 2:58 PM

tomek added inline comments.Feb 8 2022, 2:59 PM

native/android/app/src/main/java/app/comm/android/CommNotificationsHandler.java
88–89 ↗	(On Diff #9396)	We're not sending this notif yet. This handling is similar to what can be found in `handleBackgroundNotification` in `AppDelegate.mm`.

Harbormaster completed remote builds in B6631: Diff 9396.Feb 8 2022, 3:09 PM

tomek requested review of this revision.Feb 8 2022, 3:09 PM

tomek planned changes to this revision.Feb 8 2022, 5:29 PM

tomek requested review of this revision.Feb 9 2022, 12:58 PM

• karol requested changes to this revision.Feb 9 2022, 1:07 PM

• karol added inline comments.

native/android/app/src/main/java/app/comm/android/CommNotificationsHandler.java
89 ↗	(On Diff #9396)	won't this break the build? In D3132 `NetworkModule` is an empty class.

This revision now requires changes to proceed.Feb 9 2022, 1:07 PM

• karol accepted this revision.Feb 9 2022, 1:08 PM

• karol added inline comments.

native/android/app/src/main/java/app/comm/android/CommNotificationsHandler.java
89 ↗	(On Diff #9396)	Sorry I got the order of revisions wrong, all good here.

This revision is now accepted and ready to land.Feb 9 2022, 1:08 PM

Herald added a reviewer: ashoat. · View Herald TranscriptFeb 9 2022, 1:08 PM

Herald added a subscriber: • benschac. · View Herald Transcript

This revision now requires review to proceed.Feb 9 2022, 1:08 PM

ashoat accepted this revision.Feb 10 2022, 5:39 AM

ashoat added reviewers: atul, • jim.

This revision is now accepted and ready to land.Feb 10 2022, 5:39 AM

ashoat added inline comments.Feb 10 2022, 5:40 AM

native/android/app/src/main/java/app/comm/android/CommNotificationsHandler.java
89 ↗	(On Diff #9396)	It would've been good to establish a "Depends On" relationship with D3132 and D3133 here

tomek edited the summary of this revision. (Show Details)Feb 10 2022, 12:07 PM

tomek added a parent revision: D3133: [native] Add sendPong method.

tomek added inline comments.Feb 10 2022, 12:35 PM

native/android/app/src/main/java/app/comm/android/CommNotificationsHandler.java
89 ↗	(On Diff #9396)	Makes sense. I added it here and will add in the future

Closed by commit rCOMM1e26f7e47044: [native] Send PONG when PING is received. · Explain WhyFeb 10 2022, 1:06 PM

This revision was automatically updated to reflect the committed changes.

tomek added a commit: rCOMM1e26f7e47044: [native] Send PONG when PING is received.

Revision Contents
Changeset List

Path

Size

native/

android/

app/

src/

main/

java/

app/

comm/

android/

CommNotificationsHandler.java

9 lines

Diff 9517

View Options

native/android/app/src/main/java/app/comm/android/CommNotificationsHandler.java

[native] Send PONG when PING is receivedClosedPublicActions