openhab / openhab-cloud

Cloud companion for openHAB instances
Eclipse Public License 2.0
312 stars 162 forks source link

Firebase Notification for iOS is not working #463

Open lsafelix75 opened 5 days ago

lsafelix75 commented 5 days ago

I have looked through openhabcloud code and also openhab-ios. One i think realise is that the openhab-ios rely on the following code in AppDelegate.swift to eventually register IOS device token into OpenHAB-Cloud's device DB. Problem is, the following code does not always get called for some bugs related to FCM SDK (search the internet) and it is the google's Messaging device token being registered which will break the apns2 module.

func application(_ application: UIApplication, didRegisterForRemoteNotificationsWithDeviceToken deviceToken: Data) {
        dump(deviceToken as NSData)
        Messaging.messaging().apnsToken = deviceToken
    } 

Instead, i suggest to bypass apns2 module, and just let google-firebase to handle all the messages routing to IOS devices. Therefore, the following is a snippet changes that i propose in socket-io.js:

          // If we found any android devices, send notification
            if (fcmRegistrations.length > 0) {
                firebase.sendNotification(fcmRegistrations, newNotification);
            }
            // If we found any ios devices, send notification
            if (iosDeviceTokens.length > 0) {
                **use this ** _firebase.sendNotification(iosDeviceTokens, newNotification);_
                ** remove this ** **sendIosNotifications(iosDeviceTokens, newNotification);**
            }
digitaldan commented 5 days ago

Hi i'm not following what the issue is? Can you point out the FCM bugs, i'm not aware of those. Also only our test flight builds are using FCM, the production app is still using direct APNS, which we plan on deprecating very soon with the next release. Also note in the code you are referencing, that our test flight build's registrations are included in that fcmRegistrations array, and eventually we will remove iosDeviceTokens as everything will be FCM.

lsafelix75 commented 5 days ago

@digitaldan im looking at the current GitHub source code and probably has not been implemented yet in production. Anyway, the problem happens when we use this updated source on our private cloud instance as it uses sendIosNotification via APNS route BUT Openhab IOS app may not update a valid iosdeviceToken into openhabcloud userdevices table/collection.

digitaldan commented 5 days ago

I'm still not following, are you running your own build of the IOS client ? And if so are you running the latest develop branch, and you have modified it to register with your own instance and not myopenhab.org ? And you are using your own APNS key, certs, as well as a Google FCM account that is linked to your APNS account? I guess i have not heard of anyone using FCM with IOS on private instances. Maybe if you describe your setup, i might understand better.

lsafelix75 commented 5 days ago

If the direction is going toward pure FCM for both IOS and Android, and the code will be cleaned up till then, then i think no problem. But i just want to highlight the following piece of code in current OpenHAB IOS App:

extension AppDelegate: MessagingDelegate {
    func messaging(_ messaging: Messaging, didReceiveRegistrationToken fcmToken: String?) {
        os_log("My FCM token is: %{PUBLIC}@", log: .notifications, type: .info, fcmToken ?? "")
        let dataDict = [
            "deviceToken": fcmToken ?? "",
            "deviceId": UIDevice.current.identifierForVendor?.uuidString ?? "",
            "deviceName": UIDevice.current.name
        ]
        NotificationCenter.default.post(name: NSNotification.Name("apsRegistered"), object: self, userInfo: dataDict)
    }
}

If i read it correctly, the fcmToken will be updated into openhabcloud userdevices table/collection's iosDeviceToken field in subsequent calls. If we use use APNS routines in openhabcloud to send notification message instead of FCM to IOS devices, we will get "Bad token" error from the aps2 sendmessage function, because the uploaded token is FCM token and not Apple's APNS token.

digitaldan commented 4 days ago

If i read it correctly, the fcmToken will be updated into openhabcloud userdevices table/collection's iosDeviceToken field

No, it will be a fcm device token, iosDeviceToken is for the APNS connection we plan on removing in a few months once the new app is out and enough time has passed that user's upgrade. The IOS app with FCM uses a different registration API endpoint on openHAB cloud.