[system] network diagnostics for Gen 3 platforms

system/src/system_network_diagnostics.cpp

@@ -25,6 +25,7 @@
 #include "spark_wiring_fixed_point.h"
 #include "spark_wiring_platform.h"
 #include "spark_wiring_ticks.h"
+#include "system_network_diagnostics.h"
 
 #if Wiring_WiFi
 #include "spark_wiring_wifi.h"
@@ -42,6 +43,18 @@
 #define Wiring_Network 0
 #else
 
+namespace {
+
+using namespace particle;
+
+NetworkDiagnostics g_networkDiagnostics;
+
+} // namespace
+
+particle::NetworkDiagnostics* particle::NetworkDiagnostics::instance() {
+    return &g_networkDiagnostics;
+}
+
 namespace
 {
 

system/src/system_network_diagnostics.h

@@ -0,0 +1,88 @@
+/*
+ * Copyright (c) 2020 Particle Industries, Inc.  All rights reserved.
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation, either
+ * version 3 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, see <http://www.gnu.org/licenses/>.
+ */
+
+#pragma once
+
+#include "spark_wiring_diagnostics.h"
+#include "system_defs.h"
+
+namespace particle {
+
+class NetworkDiagnostics {
+public:
+    // Note: Use odd numbers to encode transitional states
+    enum Status {
+        TURNED_OFF = 0,
+        TURNING_ON = 1,
+        DISCONNECTED = 2,
+        CONNECTING = 3,
+        CONNECTED = 4,
+        DISCONNECTING = 5,
+        TURNING_OFF = 7
+    };
+
+    NetworkDiagnostics() :
+            status_(DIAG_ID_NETWORK_CONNECTION_STATUS, DIAG_NAME_NETWORK_CONNECTION_STATUS, TURNED_OFF),
+            disconnReason_(DIAG_ID_NETWORK_DISCONNECTION_REASON, DIAG_NAME_NETWORK_DISCONNECTION_REASON, NETWORK_DISCONNECT_REASON_NONE),
+            disconnCount_(DIAG_ID_NETWORK_DISCONNECTS, DIAG_NAME_NETWORK_DISCONNECTS),
+            connCount_(DIAG_ID_NETWORK_CONNECTION_ATTEMPTS, DIAG_NAME_NETWORK_CONNECTION_ATTEMPTS),
+            lastError_(DIAG_ID_NETWORK_CONNECTION_ERROR_CODE, DIAG_NAME_NETWORK_CONNECTION_ERROR_CODE) {
+    }
+
+    NetworkDiagnostics& status(Status status) {
+        status_ = status;
+        return *this;
+    }
+
+    NetworkDiagnostics& connectionAttempt() {
+        ++connCount_;
+        return *this;
+    }
+
+    NetworkDiagnostics& resetConnectionAttempts() {
+        connCount_ = 0;
+        return *this;
+    }
+
+    NetworkDiagnostics& disconnectionReason(network_disconnect_reason reason) {
+        disconnReason_ = reason;
+        return *this;
+    }
+
+    NetworkDiagnostics& disconnectedUnexpectedly() {
+        ++disconnCount_;
+        return *this;
+    }
+
+    NetworkDiagnostics& lastError(int error) {
+        lastError_ = error;
+        return *this;
+    }
+
+    static NetworkDiagnostics* instance();
+
+private:
+    // Some of the diagnostic data sources use the synchronization since they can be updated from
+    // the networking service thread
+    AtomicEnumDiagnosticData<Status> status_;
+    AtomicEnumDiagnosticData<network_disconnect_reason> disconnReason_;
+    AtomicUnsignedIntegerDiagnosticData disconnCount_;
+    SimpleUnsignedIntegerDiagnosticData connCount_;
+    SimpleIntegerDiagnosticData lastError_;
+};
+
+} // namespace particle

system/src/system_network_internal.cpp

@@ -65,21 +65,3 @@ void resetNetworkInterfaces() {
 }
 
 } // namespace particle
-
-/* FIXME: there should be a define that tells whether there is NetworkManager available
- * or not */
-#if !HAL_PLATFORM_IFAPI
-
-namespace {
-
-using namespace particle;
-
-NetworkDiagnostics g_networkDiagnostics;
-
-} // namespace
-
-particle::NetworkDiagnostics* particle::NetworkDiagnostics::instance() {
-    return &g_networkDiagnostics;
-}
-
-#endif /* !HAL_PLATFORM_IFAPI */

system/src/system_network_internal.h

@@ -90,7 +90,7 @@ void resetNetworkInterfaces();
 #include "rgbled.h"
 #include "spark_wiring_led.h"
 #include "spark_wiring_ticks.h"
-#include "spark_wiring_diagnostics.h"
+#include "system_network_diagnostics.h"
 #include "system_event.h"
 #include "system_cloud_internal.h"
 #include "system_network.h"
@@ -131,73 +131,6 @@ class NetworkStateLogger {
 #define LOG_NETWORK_STATE()
 #endif
 
-namespace particle {
-
-class NetworkDiagnostics {
-public:
-    // Note: Use odd numbers to encode transitional states
-    enum Status {
-        TURNED_OFF = 0,
-        TURNING_ON = 1,
-        DISCONNECTED = 2,
-        CONNECTING = 3,
-        CONNECTED = 4,
-        DISCONNECTING = 5,
-        TURNING_OFF = 7
-    };
-
-    NetworkDiagnostics() :
-            status_(DIAG_ID_NETWORK_CONNECTION_STATUS, DIAG_NAME_NETWORK_CONNECTION_STATUS, TURNED_OFF),
-            disconnReason_(DIAG_ID_NETWORK_DISCONNECTION_REASON, DIAG_NAME_NETWORK_DISCONNECTION_REASON, NETWORK_DISCONNECT_REASON_NONE),
-            disconnCount_(DIAG_ID_NETWORK_DISCONNECTS, DIAG_NAME_NETWORK_DISCONNECTS),
-            connCount_(DIAG_ID_NETWORK_CONNECTION_ATTEMPTS, DIAG_NAME_NETWORK_CONNECTION_ATTEMPTS),
-            lastError_(DIAG_ID_NETWORK_CONNECTION_ERROR_CODE, DIAG_NAME_NETWORK_CONNECTION_ERROR_CODE) {
-    }
-
-    NetworkDiagnostics& status(Status status) {
-        status_ = status;
-        return *this;
-    }
-
-    NetworkDiagnostics& connectionAttempt() {
-        ++connCount_;
-        return *this;
-    }
-
-    NetworkDiagnostics& resetConnectionAttempts() {
-        connCount_ = 0;
-        return *this;
-    }
-
-    NetworkDiagnostics& disconnectionReason(network_disconnect_reason reason) {
-        disconnReason_ = reason;
-        return *this;
-    }
-
-    NetworkDiagnostics& disconnectedUnexpectedly() {
-        ++disconnCount_;
-        return *this;
-    }
-
-    NetworkDiagnostics& lastError(int error) {
-        lastError_ = error;
-        return *this;
-    }
-
-    static NetworkDiagnostics* instance();
-
-private:
-    // Some of the diagnostic data sources use the synchronization since they can be updated from
-    // the networking service thread
-    AtomicEnumDiagnosticData<Status> status_;
-    AtomicEnumDiagnosticData<network_disconnect_reason> disconnReason_;
-    AtomicUnsignedIntegerDiagnosticData disconnCount_;
-    SimpleUnsignedIntegerDiagnosticData connCount_;
-    SimpleIntegerDiagnosticData lastError_;
-};
-
-} // namespace particle
-
 /**
  * Internal network interface class to provide polymorphic behavior for each
  * network type.  This is not part of the dynalib so functions can freely evolve.

system/src/system_network_manager.cpp

@@ -43,6 +43,7 @@ LOG_SOURCE_CATEGORY("system.nm")
 #include "system_event.h"
 #include "timer_hal.h"
 #include "delay_hal.h"
+#include "system_network_diagnostics.h"
 
 #define CHECKV(_expr) \
         ({ \
@@ -212,7 +213,7 @@ int NetworkManager::activateConnections() {
     return 0;
 }
 
-int NetworkManager::deactivateConnections() {
+int NetworkManager::deactivateConnections(network_disconnect_reason reason) {
     switch (state_) {
         case State::DISABLED:
         case State::IFACE_DOWN: {
@@ -223,6 +224,15 @@ int NetworkManager::deactivateConnections() {
         }
     }
 
+    if (isConnectivityAvailable()) {
+        const auto diag = NetworkDiagnostics::instance();
+        if (reason != NETWORK_DISCONNECT_REASON_NONE) {
+            diag->disconnectionReason(reason);
+            if (reason == NETWORK_DISCONNECT_REASON_ERROR || reason == NETWORK_DISCONNECT_REASON_RESET) {
+                diag->disconnectedUnexpectedly();
+            }
+        }
+    }
     transition(State::IFACE_REQUEST_DOWN);
 
     int waitingFor = 0;
@@ -365,13 +375,15 @@ void NetworkManager::transition(State state) {
     switch (state) {
         case State::DISABLED: {
             LED_SIGNAL_START(NETWORK_OFF, BACKGROUND);
-            // FIXME:
+            // FIXME: turning-off events/diagnostics
             system_notify_event(network_status, network_status_powering_off);
             system_notify_event(network_status, network_status_off);
+            NetworkDiagnostics::instance()->status(NetworkDiagnostics::TURNED_OFF);
             break;
         }
         case State::IFACE_DOWN: {
             LED_SIGNAL_START(NETWORK_ON, BACKGROUND);
+            NetworkDiagnostics::instance()->status(NetworkDiagnostics::DISCONNECTED);
             if (state_ == State::IFACE_REQUEST_DOWN) {
                 system_notify_event(network_status, network_status_disconnected);
             } else if (state_ == State::DISABLED) {
@@ -382,16 +394,27 @@ void NetworkManager::transition(State state) {
             break;
         }
         case State::IFACE_REQUEST_DOWN: {
+            if (state_ == State::IP_CONFIGURED) {
+                NetworkDiagnostics::instance()->resetConnectionAttempts();
+            }
             system_notify_event(network_status, network_status_disconnecting);
+            NetworkDiagnostics::instance()->status(NetworkDiagnostics::DISCONNECTING);
             break;
         }
         case State::IFACE_REQUEST_UP: {
             LED_SIGNAL_START(NETWORK_CONNECTING, BACKGROUND);
+            NetworkDiagnostics::instance()->status(NetworkDiagnostics::CONNECTING);
             break;
         }
         case State::IFACE_UP: {
+            if (state_ == State::IP_CONFIGURED) {
+                NetworkDiagnostics::instance()->disconnectionReason(NETWORK_DISCONNECT_REASON_ERROR);
+                NetworkDiagnostics::instance()->disconnectedUnexpectedly();
+            }
             LED_SIGNAL_START(NETWORK_CONNECTING, BACKGROUND);
             system_notify_event(network_status, network_status_connecting);
+            NetworkDiagnostics::instance()->status(NetworkDiagnostics::CONNECTING);
+            NetworkDiagnostics::instance()->connectionAttempt();
             break;
         }
         case State::IFACE_LINK_UP: {
@@ -400,6 +423,7 @@ void NetworkManager::transition(State state) {
         }
         case State::IP_CONFIGURED: {
             LED_SIGNAL_START(NETWORK_CONNECTED, BACKGROUND);
+            NetworkDiagnostics::instance()->status(NetworkDiagnostics::CONNECTED);
             if (state_ != State::IP_CONFIGURED) {
                 system_notify_event(network_status, network_status_connected);
             }
@@ -757,7 +781,13 @@ int NetworkManager::enableInterface(if_t iface) {
     return 0;
 }
 
-int NetworkManager::disableInterface(if_t iface) {
+int NetworkManager::disableInterface(if_t iface, network_disconnect_reason reason) {
+    // XXX: This method is only called on platforms with multiple network interfaces as a user
+    // request to disable a particular network interface, if there are multiple _active_ network interfaces
+    // at the moment, so 'reason' will always contain NETWORK_DISCONNECT_REASON_USER.
+    // While disabling a non-primary network interface will not in fact result in a logical disconnect,
+    // it might still be a good idea to log a user request here. We can revise this behavior later.
+    NetworkDiagnostics::instance()->disconnectionReason(reason);
     // Special case - disable all
     if (iface == nullptr) {
         populateInterfaceRuntimeState(false);

system/src/system_network_manager.h

@@ -26,6 +26,7 @@
 #include "resolvapi.h"
 #include <atomic>
 #include "intrusive_list.h"
+#include "system_defs.h"
 
 namespace particle { namespace system {
 
@@ -43,7 +44,7 @@ class NetworkManager {
     bool isNetworkingEnabled() const;
 
     int activateConnections();
-    int deactivateConnections();
+    int deactivateConnections(network_disconnect_reason reason = NETWORK_DISCONNECT_REASON_UNKNOWN);
     bool isEstablishingConnections() const;
 
     bool isConnectivityAvailable() const;
@@ -65,7 +66,7 @@ class NetworkManager {
     int clearConfiguration(if_t iface = nullptr);
 
     int enableInterface(if_t iface = nullptr);
-    int disableInterface(if_t iface = nullptr);
+    int disableInterface(if_t iface = nullptr, network_disconnect_reason reason = NETWORK_DISCONNECT_REASON_UNKNOWN);
     bool isInterfaceEnabled(if_t iface) const;
     int countEnabledInterfaces();
     int syncInterfaceStates();

system/src/system_network_manager_api.cpp

@@ -67,7 +67,7 @@ void networkDisconnectImpl(network_handle_t network, network_disconnect_reason r
     if (network != NETWORK_INTERFACE_ALL) {
         if_t iface;
         if (!if_get_by_index(network, &iface)) {
-            NetworkManager::instance()->disableInterface(iface);
+            NetworkManager::instance()->disableInterface(iface, reason);
             NetworkManager::instance()->syncInterfaceStates();
         }
     } else {
@@ -77,7 +77,7 @@ void networkDisconnectImpl(network_handle_t network, network_disconnect_reason r
 
     if ((network == NETWORK_INTERFACE_ALL ||
             NetworkManager::instance()->countEnabledInterfaces() == 0) &&
-            !NetworkManager::instance()->deactivateConnections()) {
+            !NetworkManager::instance()->deactivateConnections(reason)) {
         /* FIXME: should not loop in here */
         while (NetworkManager::instance()->getState() != NetworkManager::State::IFACE_DOWN) {
             HAL_Delay_Milliseconds(1);