orbital-systems / react-native-esp-idf-provisioning

ESP IDF provisioning and custom data library for react-native
MIT License
24 stars 5 forks source link

[ios] Errors in `scanWifiList` should not always reject promise? #22

Closed robbeman closed 8 months ago

robbeman commented 10 months ago

Hi, I'm back, sorry.

Not 100% sure if this is an issue for this package, but I want to share what I am facing and maybe hear your thoughts on this.

Here you reject the promise when an error occurs in scanWifiList: https://github.com/orbital-systems/react-native-esp-idf-provisioning/blob/3bb75bef9331e57dad1f2679a939da49e36544d6/ios/EspIdfProvisioning.swift#L130-L134

Which is great, but apparently that does not always mean it's the end.

The espressif sample app does not handle errors, it just ignores them and waits for another call of the completionHandler, as you can see in this issue. And with my device, which always returns a malformedProtobuf error before resolving this means the scan always rejects prematurely.

Possible solutions:

For testing purposes I just updated the above handler like this:

                if error != nil {
-                    reject("error", error?.description, nil)
-                    invoked = true
                    return
                }

And voila, results are coming in! Feels dirty, though.


Another possible solution would be to not use a promise, but a subscription-like strategy, so we can receive errors and still a successful response afterwards. This seems to be what the sample app is doing as well, keep the connection open and just update whenever results come in.

The error and success do seem to follow up very closely, so maybe the error will not even be visible to end users if you choose to output them.

(That would be a breaking change, I suppose.)

mateogianolio commented 10 months ago

Hey again :)

Yes, I've noticed the same issue but with provision. Our device sometimes calls "authentication failed" prematurely, then after a few seconds calls "authentication successful" which also messes up the promises.

But for scanWifiList it works every time for us. Let me ping the embedded team how they have implemented scanWifiList on the device side.

I have thought about the subscription solution as well but it makes it complicated on the app side. Would you then just ignore the first error if a success follows it? The promise solution is way cleaner so I don't really want to change it (if it's not impossible to make it work).

@mickeprag

EDIT: Maybe it's possible to have both, i.e. internally in the library it's tracked with subscriptions but externally we expose promises. And then the user can choose to ignore certain errors? Hmm...

mickeprag commented 10 months ago

The scanWifiList is not our code. We are using Espressifs component directly. So we need to look there to find out what is happening. https://github.com/espressif/esp-idf/blob/master/components/wifi_provisioning/src/wifi_scan.c

And with my device, which always returns a malformedProtobuf error before resolving this means the scan always rejects prematurely.

Is the ESP outputing any error when this happens?

robbeman commented 10 months ago

Is the ESP outputing any error when this happens?

Not as far as I can see, just goes:

I (11260) app: BLE transport: Connected!
I (11510) protocomm_nimble: mtu update event; conn_handle=0 cid=4 mtu=256
I (13700) security2: Using salt and verifier to generate public key...
I (23810) app: Secured session established!

Just so you know, I have implemented a temporary fix in our fork (see above) which is acceptable in our current case.

I'll try to keep you posted with further updates, not sure when further development will take place, but someday™️ . Thank you for your responsiveness, it was nice working with you so far. 🤝

mateogianolio commented 8 months ago

If the sample app ignores errors maybe we should do it here as well. I'll go through the sample app code when I have time!

robbeman commented 8 months ago

If the sample app ignores errors maybe we should do it here as well. I'll go through the sample app code when I have time!

Thanks, I think this is the most relevant piece of code: https://github.com/espressif/esp-idf-provisioning-ios/blob/b38391a7ca16d846af1700c902c8e87b9c35c33d/Example/ESPProvisionSample/ESPProvisionSample/Provision/ProvisionViewController.swift#L106-L116

It's a little quiet on the ESP side: https://github.com/espressif/esp-idf-provisioning-ios/issues/74