Implement infinite reconnect and cancel. (#133)

* Implement infinite reconnect with cancel.

Refactor a lot of things to support cancelling an active connection
attempt.
Significantly simplify the callback structure for the connection
progress bar.

Whilst here, fix possible null pointer dereferences in Fujifilm
service and characteristic handling.

* (#129) Reconnect without timeout

* (#129) Enhance Warning

* Remove stale warning in README.

---------

Co-authored-by: Hijae Song <[email protected]>

README.md

@@ -146,6 +146,18 @@ WARNING:
 * mobile device connections are extremely finnicky
 * multi-connect involving mobile devices is not well tested and can easily crash
 
+### Infinite-ReConnect
+
+This is useful for using furble as a passive, always on GPS data source.
+With this, the camera will attempt to reconnect indefinitely.
+You don't need to turn on this setting if you are actively using furble.
+
+To use:
+* Enable `Settings->Infinite-ReConnect`
+
+WARNING:
+* this will not be kind to battery life
+
 ## Motivation
 
 I found current smartphone apps for basic wireless remote shutter control to be

include/furble_control.h

@@ -12,6 +12,7 @@ typedef enum {
   CONTROL_CMD_FOCUS_PRESS,
   CONTROL_CMD_FOCUS_RELEASE,
   CONTROL_CMD_GPS_UPDATE,
+  CONTROL_CMD_CONNECT,
   CONTROL_CMD_DISCONNECT,
   CONTROL_CMD_ERROR
 } control_cmd_t;
@@ -28,11 +29,10 @@ class Control {
     Camera *getCamera(void);
     control_cmd_t getCommand(void);
     void sendCommand(control_cmd_t cmd);
-    const Camera::gps_t &getGPS(void);
-    const Camera::timesync_t &getTimesync(void);
-
     void updateGPS(Camera::gps_t &gps, Camera::timesync_t &timesync);
 
+    void task(void);
+
    private:
     QueueHandle_t m_Queue = NULL;
     Furble::Camera *m_Camera = NULL;
@@ -61,13 +61,18 @@ class Control {
   /**
    * Are all active cameras still connected?
    */
-  bool isConnected(void);
+  bool allConnected(void);
 
   /**
    * Get list of connected targets.
    */
   const std::vector<std::unique_ptr<Control::Target>> &getTargets(void);
 
+  /**
+   * Connect to the specified camera.
+   */
+  void connect(Camera *, esp_power_level_t power);
+
   /**
    * Disconnect all connected cameras.
    */
@@ -81,6 +86,10 @@ class Control {
  private:
   QueueHandle_t m_Queue = NULL;
   std::vector<std::unique_ptr<Control::Target>> m_Targets;
+
+  // Camera connects are serialised, the following track the last attempt
+  Camera *m_ConnectCamera = nullptr;
+  esp_power_level_t m_Power;
 };
 
 };  // namespace Furble

include/furble_ui.h

@@ -5,7 +5,7 @@
 
 struct FurbleCtx {
   Furble::Control *control;
-  bool reconnected;
+  bool cancelled;
 };
 
 void vUITask(void *param);

include/settings.h

@@ -9,6 +9,9 @@
 void settings_menu_tx_power(void);
 esp_power_level_t settings_load_esp_tx_power(void);
 
+bool settings_load_reconnect(void);
+void settings_save_reconnect(bool reconnect);
+
 bool settings_load_gps(void);
 void settings_menu_gps(void);
 

lib/M5ez/src/M5ez.cpp

@@ -1058,10 +1058,14 @@ void M5ez::begin() {
   ez.settings.begin();
 }
 
-void M5ez::yield() {
+void M5ez::yield(bool events) {
   vTaskDelay(1);  // allow lower priority tasks to run
   ::yield();      // execute the Arduino yield in the root namespace
   M5.update();
+  if (!events) {
+    return;
+  }
+
   for (uint8_t n = 0; n < _events.size(); n++) {
     if (millis() > _events[n].when) {
       uint16_t r = (_events[n].function)(_events[n].context);

lib/M5ez/src/M5ez.h

@@ -503,7 +503,7 @@ class M5ez {
 
   static void begin();
 
-  static void yield();
+  static void yield(bool events = true);
 
   static void addEvent(uint16_t (*function)(void *), void *context = nullptr, uint32_t when = 1);
   static void removeEvent(uint16_t (*function)(void *context));

lib/furble/Camera.cpp

@@ -12,13 +12,16 @@ Camera::~Camera() {
   NimBLEDevice::deleteClient(m_Client);
 }
 
-bool Camera::connect(esp_power_level_t power, progressFunc pFunc, void *pCtx) {
+bool Camera::connect(esp_power_level_t power) {
   // try extending range by adjusting connection parameters
-  bool connected = this->connect(pFunc, pCtx);
+  bool connected = this->connect();
   if (connected) {
     // Set BLE transmit power after connection is established.
     NimBLEDevice::setPower(power);
     m_Client->updateConnParams(m_MinInterval, m_MaxInterval, m_Latency, m_Timeout);
+    m_Connected = true;
+  } else {
+    m_Connected = false;
   }
 
   return connected;
@@ -44,14 +47,12 @@ const NimBLEAddress &Camera::getAddress(void) const {
   return m_Address;
 }
 
-void Camera::updateProgress(progressFunc pFunc, void *ctx, float value) {
-  if (pFunc != nullptr) {
-    (pFunc)(ctx, value);
-  }
+float Camera::getConnectProgress(void) const {
+  return m_Progress.load();
 }
 
 bool Camera::isConnected(void) const {
-  return m_Client->isConnected();
+  return m_Connected && m_Client->isConnected();
 }
 
 }  // namespace Furble

lib/furble/Camera.h

@@ -1,6 +1,8 @@
 #ifndef CAMERA_H
 #define CAMERA_H
 
+#include <atomic>
+
 #include <NimBLEAddress.h>
 #include <NimBLEClient.h>
 #include <NimBLEDevice.h>
@@ -9,9 +11,6 @@
 
 #define MAX_NAME (64)
 
-// Progress update function
-typedef void(progressFunc(void *, float));
-
 namespace Furble {
 
 /**
@@ -55,7 +54,7 @@ class Camera {
   /**
    * Wrapper for protected pure virtual Camera::connect().
    */
-  bool connect(esp_power_level_t power, progressFunc pFunc = nullptr, void *pCtx = nullptr);
+  bool connect(esp_power_level_t power);
 
   /**
    * Disconnect from the target.
@@ -111,8 +110,11 @@ class Camera {
 
   const NimBLEAddress &getAddress(void) const;
 
+  float getConnectProgress(void) const;
+
  protected:
   Camera(Type type);
+  std::atomic<float> m_Progress;
 
   /**
    * Connect to the target camera such that it is ready for shutter control.
@@ -122,13 +124,12 @@ class Camera {
    *
    * @return true if the client is now ready for shutter control
    */
-  virtual bool connect(progressFunc pFunc = nullptr, void *pCtx = nullptr) = 0;
+  virtual bool connect(void) = 0;
 
   NimBLEAddress m_Address = NimBLEAddress{};
   NimBLEClient *m_Client;
   std::string m_Name;
-
-  void updateProgress(progressFunc pFunc, void *ctx, float value);
+  bool m_Connected = false;
 
  private:
   const uint16_t m_MinInterval = BLE_GAP_INITIAL_CONN_ITVL_MIN;

lib/furble/CanonEOS.cpp

@@ -75,7 +75,7 @@ bool CanonEOS::write_prefix(NimBLEClient *pClient,
  * The EOS uses the 'just works' BLE bonding to pair, all bond management is
  * handled by the underlying NimBLE and ESP32 libraries.
  */
-bool CanonEOS::connect(progressFunc pFunc, void *pCtx) {
+bool CanonEOS::connect(void) {
   if (NimBLEDevice::isBonded(m_Address)) {
     // Already bonded? Assume pair acceptance!
     m_PairResult = CANON_EOS_PAIR_ACCEPT;
@@ -90,14 +90,14 @@ bool CanonEOS::connect(progressFunc pFunc, void *pCtx) {
   }
 
   ESP_LOGI(LOG_TAG, "Connected");
-  updateProgress(pFunc, pCtx, 10.0f);
+  m_Progress = 10.0f;
 
   ESP_LOGI(LOG_TAG, "Securing");
   if (!m_Client->secureConnection()) {
     return false;
   }
   ESP_LOGI(LOG_TAG, "Secured!");
-  updateProgress(pFunc, pCtx, 20.0f);
+  m_Progress = 20.0f;
 
   NimBLERemoteService *pSvc = m_Client->getService(CANON_EOS_SVC_IDEN_UUID);
   if (pSvc) {
@@ -116,37 +116,36 @@ bool CanonEOS::connect(progressFunc pFunc, void *pCtx) {
                     (uint8_t *)name, strlen(name)))
     return false;
 
-  updateProgress(pFunc, pCtx, 30.0f);
+  m_Progress = 30.0f;
 
   ESP_LOGI(LOG_TAG, "Identifying 2!");
   if (!write_prefix(m_Client, CANON_EOS_SVC_IDEN_UUID, CANON_EOS_CHR_IDEN_UUID, 0x03, m_Uuid.uint8,
                     UUID128_LEN))
     return false;
 
-  updateProgress(pFunc, pCtx, 40.0f);
+  m_Progress = 40.0f;
 
   ESP_LOGI(LOG_TAG, "Identifying 3!");
   if (!write_prefix(m_Client, CANON_EOS_SVC_IDEN_UUID, CANON_EOS_CHR_IDEN_UUID, 0x04,
                     (uint8_t *)name, strlen(name)))
     return false;
 
-  updateProgress(pFunc, pCtx, 50.0f);
+  m_Progress = 50.0f;
 
   ESP_LOGI(LOG_TAG, "Identifying 4!");
 
   uint8_t x = 0x02;
   if (!write_prefix(m_Client, CANON_EOS_SVC_IDEN_UUID, CANON_EOS_CHR_IDEN_UUID, 0x05, &x, 1))
     return false;
 
-  updateProgress(pFunc, pCtx, 60.0f);
+  m_Progress = 60.0f;
 
   ESP_LOGI(LOG_TAG, "Identifying 5!");
 
   // Give the user 60s to confirm/deny pairing
   ESP_LOGI(LOG_TAG, "Waiting for user to confirm/deny pairing.");
   for (unsigned int i = 0; i < 60; i++) {
-    float progress = 70.0f + (float(i) / 6.0f);
-    updateProgress(pFunc, pCtx, progress);
+    m_Progress = 70.0f + (float(i) / 6.0f);
     if (m_PairResult != 0x00) {
       break;
     }
@@ -166,7 +165,7 @@ bool CanonEOS::connect(progressFunc pFunc, void *pCtx) {
   if (!write_value(m_Client, CANON_EOS_SVC_IDEN_UUID, CANON_EOS_CHR_IDEN_UUID, &x, 1))
     return false;
 
-  updateProgress(pFunc, pCtx, 80.0f);
+  m_Progress = 80.0f;
 
   ESP_LOGI(LOG_TAG, "Switching mode!");
 
@@ -176,7 +175,7 @@ bool CanonEOS::connect(progressFunc pFunc, void *pCtx) {
     return false;
 
   ESP_LOGI(LOG_TAG, "Done!");
-  updateProgress(pFunc, pCtx, 100.0f);
+  m_Progress = 100.0f;
 
   return true;
 }
@@ -207,6 +206,8 @@ void CanonEOS::updateGeoData(const gps_t &gps, const timesync_t &timesync) {
 }
 
 void CanonEOS::disconnect(void) {
+  m_Progress = 0.0f;
+  m_Connected = false;
   m_Client->disconnect();
 }
 

lib/furble/CanonEOS.h

@@ -55,7 +55,7 @@ class CanonEOS: public Camera {
                     uint8_t *data,
                     size_t length);
 
-  bool connect(progressFunc pFunc = nullptr, void *pCtx = nullptr) override;
+  bool connect(void) override;
   void shutterPress(void) override;
   void shutterRelease(void) override;
   void focusPress(void) override;