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

All registration of callbacks for various "on connection attempt" failures #243

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

aentinger
Copy link
Collaborator

@aentinger aentinger commented Apr 21, 2021

Requires arduino-libraries/Arduino_ConnectionHandler#57.

#include "arduino_secrets.h"
#include "thingProperties.h"

void onPhyFail()
{
  Serial.println("[Connection Error] Could not connect to WiFi - check SSID and PASS");
}
void onMqttConnectFail()
{
  Serial.println("[Connection Error] Provision your device or verify MQTT server settings");
}
void onMqttSubscribeFail()
{
  Serial.println("[Connection Error] Assign a device to your thing and verify thing id!");
}
 
void setup()
{
  Serial.begin(9600);
  while (!Serial) { }

  initProperties();

  setDebugMessageLevel(DBG_VERBOSE);

  ArduinoCloud.begin(ArduinoIoTPreferredConnection);

  ArduinoCloud.onPhyConnectionFailure(onPhyFail);
  ArduinoCloud.onMQTTConnectionFailure(onMqttConnectFail);
  ArduinoCloud.onMQTTSubscriptionFailure(onMqttSubscribeFail);
}

void loop()
{
  ArduinoCloud.update();
}

…nnection' failure types:

- Can not connect to the network (WiFi, GSM, ...)
- Can not connect to MQTT broker
- Can not subscribe to MQTT topics.
@aentinger aentinger added the type: enhancement Proposed improvement label Apr 21, 2021
@aentinger aentinger self-assigned this Apr 21, 2021
@aentinger aentinger added this to In progress in Arduino IoT Cloud via automation Apr 21, 2021
@codecov-commenter
Copy link

codecov-commenter commented Apr 21, 2021

Codecov Report

Patch and project coverage have no change.

Comparison is base (1919728) 95.15% compared to head (62be681) 95.15%.
Report is 434 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #243   +/-   ##
=======================================
  Coverage   95.15%   95.15%           
=======================================
  Files          24       24           
  Lines         867      867           
=======================================
  Hits          825      825           
  Misses         42       42           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@ubidefeo
Copy link
Collaborator

I like the solution
Out of zeal I might not use MQTT as a word but it's just a preference.
Maybe "Broker" is less restrictive and still conveys the information.
@alranel what do you think?

@alranel
Copy link
Member

alranel commented Apr 22, 2021

Yes @ubidefeo, I agree! MQTT is not a user-facing concept for Arduino Cloud users, at least for now. It's a protocol used under the hood, so I would not use it in the naming of the hooks.

In my notes I mapped these events that would be useful to expose via API so that they can be exposed in the UI of a device (display, LEDs, buzzer...):

  1. configured WiFi network not found
    • "The configured WiFi network is not available: check the SSID you entered"
  2. WiFi authentication failed
    • "Failed to authenticate to WiFi: check the password"
  3. WiFi is connected but IoT Cloud is not reachable (IP, MQTT down…)
    • "Your Internet connection seems to be down OR the Arduino Cloud might be temporarily unavailable: check your connection"
  4. not connected to IoT Cloud / authentication issues
    • "Your device is not correctly provisioned in IoT Cloud: follow the provisioning wizard"
  5. not connected to IoT Cloud / failed to subscribe because of thing ID was not found
    • "Configuration issue: check the configuration of your thing in IoT Cloud"
  6. not connected to Iot Cloud / variables not found
    • "Configuration issue: check the variables configured for this thing in IoT Cloud"

The proposed onPhyFail() hook would cover 1 and 2, but ideally we should have distinct hooks to display a more actionable error message. Is this possible?

The proposed onMqttConnectFail() seems to mix 3 and 4, while onMqttSubscribeFail() seems to cover 5 and 6.

@aentinger Do you think we can achieve more granularity like this schema?

@aentinger
Copy link
Collaborator Author

aentinger commented Apr 22, 2021

I see why you'd desire such a fine-grained granularity as it would for extremely user-friendly error messages.

Right now 1) and 2) can not be distinguished as for the AIOTC firmware stack it's simply a failure to connect. One could though distinguish failure case 1) by checking the availability of the WIFI_SSID defined in arduino_secrets.h before running the firmware stack. This could be potentially integrated in the connection handler but is not straight-forward as the other network types (GSM, LoRa) don't have such a discovery step.

Now 3) and 4) unfortunately really can't be distinguished by technical means right now. Maybe there is a way but I'll have to dig very deep in all concerned components (the SSL stack is kind of opaque, maybe its possible to check for "aliveness" of the server via a ping - be aware though that such steps further complicate the connection procedure and might introduce other errors).

. 6) Isn't detectable at all. If your variables don't exist in the cloud there's no error feedback mechanism. It would fall to the cloud to notify the user if properties are sent under a thing-id which have not been configured.

@aentinger
Copy link
Collaborator Author

Lastly: I agree, MQTT is a bit technical (but not overly so, it's quite common sense among many people, even if not die-hard programmers). I'm open for suggestions for other callback names though.

@per1234 per1234 added the topic: code Related to content of the project itself label Jun 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
topic: code Related to content of the project itself type: enhancement Proposed improvement
Projects
Development

Successfully merging this pull request may close these issues.

None yet

5 participants