ACM MVP Behavior changes

system/inc/system_cloud.h

@@ -126,7 +126,8 @@ const CloudVariableTypeDouble DOUBLE;
  */
 enum CloudDisconnectFlag {
     CLOUD_DISCONNECT_GRACEFULLY = 0x01, ///< Disconnect gracefully.
-    CLOUD_DISCONNECT_DONT_CLOSE = 0x02 ///< Do not close the socket.
+    CLOUD_DISCONNECT_DONT_CLOSE = 0x02, ///< Do not close the socket.
+    CLOUD_DISCONNECT_RECONNECT_IMMEDIATELY = 0x03 ///< Close the socket, but leave the cloud connect flag set
 };
 
 /**
@@ -313,7 +314,8 @@ bool spark_cloud_flag_auto_connect(void);
 typedef enum spark_cloud_disconnect_option_flag {
     SPARK_CLOUD_DISCONNECT_OPTION_GRACEFUL = 0x01, ///< The `graceful` option is set.
     SPARK_CLOUD_DISCONNECT_OPTION_TIMEOUT = 0x02, ///< The `timeout` option is set.
-    SPARK_CLOUD_DISCONNECT_OPTION_CLEAR_SESSION = 0x04 ///< The `clear_session` option is set.
+    SPARK_CLOUD_DISCONNECT_OPTION_CLEAR_SESSION = 0x04, ///< The `clear_session` option is set.
+    SPARK_CLOUD_DISCONNECT_OPTION_RECONNECT_IMMEDIATELY = 0x08 ///< The `reconnect_immediately` option is set.
 } spark_cloud_disconnect_option_flag;
 
 /**
@@ -325,6 +327,7 @@ typedef struct spark_cloud_disconnect_options {
     uint8_t graceful; ///< Enables graceful disconnection if set to a non-zero value.
     uint32_t timeout; ///< Maximum time in milliseconds to wait for message acknowledgements.
     uint8_t clear_session; ///< Clears the session after disconnecting if set to a non-zero value.
+    uint8_t reconnect_immediately;
 } spark_cloud_disconnect_options;
 
 /**
@@ -351,8 +354,7 @@ typedef enum spark_connection_property {
     SPARK_CLOUD_MAX_EVENT_DATA_SIZE = 3, ///< Maximum size of event data (get).
     SPARK_CLOUD_MAX_VARIABLE_VALUE_SIZE = 4, ///< Maximum size of a variable value (get).
     SPARK_CLOUD_MAX_FUNCTION_ARGUMENT_SIZE = 5, ///< Maximum size of a function call argument (get).
-    SPARK_CLOUD_BIND_NETWORK_INTERFACE = 6, ///< The cloud connection should only use a specified network interface
-    SPARK_CLOUD_GET_NETWORK_INTERFACE = 7 ///< Which interface is being used for the current cloud connection
+    SPARK_CLOUD_GET_NETWORK_INTERFACE = 6 ///< Which interface is being used for the current cloud connection
 } spark_connection_property;
 
 int spark_set_connection_property(unsigned property, unsigned value, const void* data, void* reserved);

system/inc/system_task.h

@@ -55,8 +55,6 @@ inline void Spark_Idle() { Spark_Idle_Events(false); }
 void SPARK_WLAN_Loop(void) __attribute__ ((deprecated("Please use Particle.process() instead.")));
 inline void SPARK_WLAN_Loop(void) { spark_process(); }
 
-void disconnect_cloud();
-
 extern volatile uint8_t SPARK_WLAN_RESET;
 extern volatile uint8_t SPARK_WLAN_SLEEP;
 extern volatile uint8_t SPARK_WLAN_CONNECT_RESTORE;

system/src/system_cloud.cpp

@@ -302,10 +302,6 @@ int spark_set_connection_property(unsigned property, unsigned value, const void*
         const auto r = spark_protocol_set_connection_property(sp, property, value, d, reserved);
         return spark_protocol_to_system_error(r);
     }
-    case SPARK_CLOUD_BIND_NETWORK_INTERFACE: {
-        CloudConnectionSettings::instance()->setBoundInterface((network_interface_t)value);
-        return 0;
-    }
 
     default:
         return SYSTEM_ERROR_INVALID_ARGUMENT;
@@ -331,16 +327,9 @@ int spark_get_connection_property(unsigned property, void* data, size_t* size, v
             return SYSTEM_ERROR_INVALID_STATE;
         }
         return getConnectionProperty(protocol::Connection::MAX_FUNCTION_ARGUMENT_SIZE, data, size);
-    case SPARK_CLOUD_BIND_NETWORK_INTERFACE: {
-        if (*size >= sizeof(network_interface_t)) {
-            *((network_interface_t*)data) = CloudConnectionSettings::instance()->getBoundInterface();
-            return 0;    
-        }
-        return SYSTEM_ERROR_INVALID_ARGUMENT;
-    }
     case SPARK_CLOUD_GET_NETWORK_INTERFACE: {
         if (*size >= sizeof(network_interface_t)) {
-#if HAL_PLATFORM_AUTOMATIC_CONNECTION_MANAGEMENT
+#if HAL_PLATFORM_IFAPI
             *((network_interface_t*)data) = ConnectionManager::instance()->getCloudConnectionNetwork();
 #else
             *((network_interface_t*)data) = NETWORK_INTERFACE_ALL;

system/src/system_cloud_connection_posix.cpp

@@ -166,7 +166,6 @@ int system_cloud_connect(int protocol, const ServerAddress* address, sockaddr* s
                 break;
             }
         }
-        LOG(INFO, "Cloud socket=%d, connecting to %s#%u", s, serverHost, serverPort);
 
         /* We are using fixed source port only for IPv6 connections */
         if (protocol == IPPROTO_UDP && a->ai_family == AF_INET6) {
@@ -203,13 +202,19 @@ int system_cloud_connect(int protocol, const ServerAddress* address, sockaddr* s
 
         network_interface_t cloudInterface = particle::system::ConnectionManager::instance()->selectCloudConnectionNetwork();
 
+        LOG(INFO, "Cloud socket=%d, connecting to %s#%u using if %d", s, serverHost, serverPort, cloudInterface);
+
         if (cloudInterface != NETWORK_INTERFACE_ALL) {    
             // Bind to specific netif
             struct ifreq ifr = {};
             if_index_to_name(cloudInterface, ifr.ifr_name);
 
             auto sockOptRet = sock_setsockopt(s, SOL_SOCKET, SO_BINDTODEVICE, &ifr, sizeof(ifr));
-            LOG(TRACE, "%d Bound cloud socket to lwip interface %s", sockOptRet, ifr.ifr_name);
+            if (sockOptRet) {
+                LOG(WARN, "Failed to bind cloud socket to interface %u error: %d", cloudInterface, sockOptRet);
+            } else {
+                LOG(INFO, "Bound cloud socket to lwip if %u (\"%s\")", cloudInterface, ifr.ifr_name);
+            }
         }
 
         /* FIXME: timeout for TCP */
@@ -314,8 +319,27 @@ int system_cloud_disconnect(int flags)
 int system_cloud_send(const uint8_t* buf, size_t buflen, int flags)
 {
     (void)flags;
-    LOG_DEBUG(TRACE, "TX s_state.socket %d buflen %d", s_state.socket, buflen);
-    int r = sock_send(s_state.socket, buf, buflen, 0);
+    int r = 0;
+
+    auto netIfIndex = particle::system::ConnectionManager::instance()->getCloudConnectionNetwork();
+    if (netIfIndex) {
+        if_t ifHandle = 0;
+        if_get_by_index(netIfIndex, &ifHandle);
+        unsigned int ifFlags = 0;
+        if_get_flags(ifHandle, &ifFlags);
+        LOG_DEBUG(TRACE, "Bound cloud netif idx %d flags 0x%0x (IFF_RUNNING %d)", netIfIndex, ifFlags, (ifFlags & IFF_RUNNING)!=0);
+
+        if (!(ifFlags & IFF_RUNNING)) {
+            LOG(WARN, "Netif %d not running. if flags %0x", netIfIndex, ifFlags);
+            r = ERR_CONN;     
+        }
+    }
+
+    if (!r) {
+        LOG_DEBUG(TRACE, "TX s_state.socket %d buflen %d", s_state.socket, buflen);
+        r = sock_send(s_state.socket, buf, buflen, 0);
+    }
+
     if (r < 0) {
         if (errno == ENOMEM) {
             /* Not an error */

system/src/system_cloud_internal.h

@@ -138,12 +138,13 @@ class CloudConnectionSettings {
     static const bool DEFAULT_DISCONNECT_GRACEFULLY = false;
     static const unsigned DEFAULT_DISCONNECT_TIMEOUT = 30000;
     static const bool DEFAULT_DISCONNECT_CLEAR_SESSION = false;
+    static const bool DEFAULT_RECONNECT = false;
 
     CloudConnectionSettings() :
             defaultDisconnectTimeout_(DEFAULT_DISCONNECT_TIMEOUT),
             defaultDisconnectGracefully_(DEFAULT_DISCONNECT_GRACEFULLY),
             defaultDisconnectClearSession_(DEFAULT_DISCONNECT_CLEAR_SESSION),
-            boundInterface_(NETWORK_INTERFACE_ALL) {
+            defaultDisconnectReconnect_(DEFAULT_RECONNECT) {
     }
 
     void setDefaultDisconnectOptions(const CloudDisconnectOptions& options) {
@@ -156,6 +157,9 @@ class CloudConnectionSettings {
         if (options.isClearSessionSet()) {
             defaultDisconnectClearSession_ = options.clearSession();
         }
+        if (options.isReconnectSet()) {
+            defaultDisconnectReconnect_ = options.reconnect();
+        }
     }
 
     bool defaultDisconnectGracefully() const {
@@ -190,19 +194,16 @@ class CloudConnectionSettings {
         } else {
             result.clearSession(defaultDisconnectClearSession_);
         }
+        if (pending.isReconnectSet()) {
+            result.reconnect(pending.reconnect());
+        } else {
+            result.reconnect(defaultDisconnectReconnect_);
+        }
         return result;
     }
 
     static CloudConnectionSettings* instance();
 
-    void setBoundInterface(network_interface_t network) {
-        boundInterface_ = network;
-    }
-
-    network_interface_t getBoundInterface() {
-        return boundInterface_;
-    }
-
 private:
     SimpleAtomicFlagMutex mutex_;
     // Default disconnection options can only be set in the context of the system thread.
@@ -212,7 +213,7 @@ class CloudConnectionSettings {
     bool defaultDisconnectClearSession_;
     // Pending disconnection options are set atomically and guarded by a spinlock
     CloudDisconnectOptions pendingDisconnectOptions_;
-    network_interface_t boundInterface_;
+    bool defaultDisconnectReconnect_;
 };
 
 // Use this function instead of Particle.publish() in the system code

system/src/system_connection_manager.cpp

@@ -96,7 +96,13 @@ ConnectionManager* ConnectionManager::instance() {
 void ConnectionManager::setPreferredNetwork(network_handle_t network, bool preferred) {
     if (preferred) {
         if (network != NETWORK_INTERFACE_ALL) {
-            preferredNetwork_ = network;    
+            preferredNetwork_ = network;
+
+            // If cloud is already connected, and a preferred network is set, and it is up, move cloud connection to it immediately
+            if (spark_cloud_flag_connected() && network_ready(spark::Network.from(preferredNetwork_), 0, nullptr)) {
+                auto options = CloudDisconnectOptions().graceful(true).reconnect(true).toSystemOptions();
+                spark_cloud_disconnect(&options, nullptr);
+            }
         }
     } else {
         if (network == preferredNetwork_ || network == NETWORK_INTERFACE_ALL) {
@@ -124,25 +130,14 @@ network_handle_t ConnectionManager::getCloudConnectionNetwork() {
 network_handle_t ConnectionManager::selectCloudConnectionNetwork() {
     network_handle_t bestNetwork = NETWORK_INTERFACE_ALL;
 
-    // 1: If there is a bound network connection. Do not use anything else, regardless of network state
-    network_handle_t boundNetwork = NETWORK_INTERFACE_ALL;
-    size_t n = sizeof(boundNetwork);
-    spark_get_connection_property(SPARK_CLOUD_BIND_NETWORK_INTERFACE, &boundNetwork, &n, nullptr);
-
-    if (boundNetwork != NETWORK_INTERFACE_ALL) {
-        LOG_DEBUG(TRACE, "Using bound network: %lu", boundNetwork);
-        return boundNetwork;
-    }
-
-    // 2: If no bound network, use preferred network
     if (preferredNetwork_ != NETWORK_INTERFACE_ALL && network_ready(spark::Network.from(preferredNetwork_), 0, nullptr)) {
         LOG_DEBUG(TRACE, "Using preferred network: %lu", preferredNetwork_);
         return preferredNetwork_;
     }
 
-    // 3: If no preferred network, use the 'best' network based on criteria
-    // 3.1: Network is ready: ie configured + connected (see ipv4 routable hook)
-    // 3.2: Network has best criteria based on network tester results
+    // If no preferred network, use the 'best' network based on criteria
+    // Network is ready: ie configured + connected (see ipv4 routable hook)
+    // Network has best criteria based on network tester results
     for (auto& i: bestNetworks_) {
         if (network_ready(spark::Network.from(i), 0, nullptr)) {
             LOG_DEBUG(TRACE, "Using best network: %lu", i);
@@ -236,7 +231,7 @@ int ConnectionTester::sendTestPacket(ConnectionMetrics* metrics) {
     if (metrics->txPacketCount == metrics->rxPacketCount || 
         (millis() > metrics->txPacketStartMillis + REACHABILITY_TEST_PACKET_TIMEOUT_MS)) {
 
-        CHECK(generateTestPacket(metrics));
+        generateTestPacket(metrics);
 
         int r = sock_send(metrics->socketDescriptor, metrics->txBuffer, metrics->testPacketSize, 0);
         if (r > 0) {
@@ -278,12 +273,6 @@ int ConnectionTester::receiveTestPacket(ConnectionMetrics* metrics) {
 };
 
 int ConnectionTester::generateTestPacket(ConnectionMetrics* metrics) {
-    auto network = spark::Network.from(metrics->interface);
-    if (!network) {
-        LOG(ERROR, "No Network associated with interface %d", metrics->interface);
-        return SYSTEM_ERROR_BAD_DATA;
-    }
-
     unsigned packetDataLength = random(1, REACHABILITY_MAX_PAYLOAD_SIZE);
 
     DTLSPlaintext_t msg = {
@@ -331,7 +320,7 @@ int ConnectionTester::pollSockets(struct pollfd* pfds, int socketCount) {
             receiveTestPacket(connection);
         }
         if (pfds[i].revents & POLLOUT) {
-            sendTestPacket(connection);
+            CHECK(sendTestPacket(connection));
         }
     }
     return 0;
@@ -380,7 +369,12 @@ int ConnectionTester::testConnections() {
         // For each network interface to test, create + open a socket with the retrieved server address
         // If any of the sockets fail to be created + opened with this server address, return an error
         for (auto& connectionMetrics: metrics_) {
-            if (!network_ready(spark::Network.from(connectionMetrics.interface),0,nullptr)) {
+            auto network = spark::Network.from(connectionMetrics.interface);
+            if (network == spark::Network) {
+                LOG(ERROR, "No Network associated with interface %d", connectionMetrics.interface);
+                return SYSTEM_ERROR_BAD_DATA;
+            }
+            if (!network_ready(network, 0, nullptr)) {
                 LOG_DEBUG(TRACE,"%s not ready, skipping test", netifToName(connectionMetrics.interface));
                 continue;
             }
@@ -487,13 +481,13 @@ const Vector<ConnectionMetrics> ConnectionTester::getConnectionMetrics(){
 const Vector<network_interface_t> ConnectionTester::getSupportedInterfaces() {
     const Vector<network_interface_t> interfaceList = { 
 #if HAL_PLATFORM_ETHERNET
-    static_cast<network_interface_t>(NetworkDiagnosticsInterface::ETHERNET),
+    NETWORK_INTERFACE_ETHERNET,
 #endif
 #if HAL_PLATFORM_WIFI 
-    static_cast<network_interface_t>(NetworkDiagnosticsInterface::WIFI_STA),
+    NETWORK_INTERFACE_WIFI_STA,
 #endif
 #if HAL_PLATFORM_CELLULAR
-    static_cast<network_interface_t>(NetworkDiagnosticsInterface::CELLULAR)
+    NETWORK_INTERFACE_CELLULAR
 #endif
     };
 

system/src/system_network_manager.cpp

@@ -51,6 +51,8 @@ LOG_SOURCE_CATEGORY("system.nm")
 #include "control/common.h"
 #include "control/network.h"
 #include <unistd.h>
+#include "system_connection_manager.h"
+#include "spark_wiring_cloud.h"
 
 #define CHECKV(_expr) \
         ({ \
@@ -536,6 +538,11 @@ void NetworkManager::handleIfState(if_t iface, const struct if_event* ev) {
 }
 
 void NetworkManager::handleIfLink(if_t iface, const struct if_event* ev) {
+    uint8_t netIfIndex = 0;
+    if_get_index(iface, &netIfIndex);
+    bool disconnectCloud = false;
+    auto options = CloudDisconnectOptions().reconnect(true);
+
     if (ev->ev_if_link->state) {
         /* Interface link state changed to UP */
         NetworkInterfaceConfig conf;
@@ -609,11 +616,25 @@ void NetworkManager::handleIfLink(if_t iface, const struct if_event* ev) {
         } else if (state_ == State::IP_CONFIGURED || state_ == State::IFACE_LINK_UP) {
             refreshIpState();
         }
+
+        // If the preferred network becomes available, close the cloud connection to force migration to this network
+        if (ConnectionManager::instance()->getPreferredNetwork() == netIfIndex) {
+            LOG(INFO, "Preferred network %u available, moving cloud connection", netIfIndex);
+            options.graceful(true);
+            disconnectCloud = true;
+        }
+
     } else {
         // Disable by default
         if_clear_xflags(iface, IFXF_DHCP);
         resetInterfaceProtocolState(iface);
 
+        // If this is the current cloud connection, disconnect the cloud, but reconnect immediately on any other available interface
+        if (ConnectionManager::instance()->getCloudConnectionNetwork() == netIfIndex) {
+            LOG(WARN, "Cloud connection interface %d link state down, disconnecting", netIfIndex);
+            disconnectCloud = true;
+        }
+
         clearDnsConfiguration(iface);
         /* Interface link state changed to DOWN */
         if (state_ == State::IP_CONFIGURED || state_ == State::IFACE_LINK_UP) {
@@ -624,6 +645,12 @@ void NetworkManager::handleIfLink(if_t iface, const struct if_event* ev) {
             }
         }
     }
+
+    if (spark_cloud_flag_connected() && disconnectCloud) {
+        auto systemOptions = options.toSystemOptions();
+        spark_cloud_disconnect(&systemOptions, nullptr);
+    }
+
     forceCloudPingIfConnected();
 }
 

system/src/system_task.cpp

@@ -692,6 +692,10 @@ void cloud_disconnect(unsigned flags, cloud_disconnect_reason cloudReason, netwo
             spark_cloud_socket_disconnect(graceful);
         }
 
+        if (opts.reconnect()) {
+            spark_cloud_flag_connect();
+        }
+
         // Note: Invoking the protocol's DISCONNECT or TERMINATE command cancels the ongoing firmware
         // if the device is being updated OTA, so there's no need to explicitly cancel the update here
         SPARK_CLOUD_CONNECTED = 0;