diff --git a/system/src/system_network_diagnostics.cpp b/system/src/system_network_diagnostics.cpp index 7c6e969380..f6b9cd8974 100644 --- a/system/src/system_network_diagnostics.cpp +++ b/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 { diff --git a/system/src/system_network_diagnostics.h b/system/src/system_network_diagnostics.h new file mode 100644 index 0000000000..6fc16e8b67 --- /dev/null +++ b/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 . + */ + +#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_; + AtomicEnumDiagnosticData disconnReason_; + AtomicUnsignedIntegerDiagnosticData disconnCount_; + SimpleUnsignedIntegerDiagnosticData connCount_; + SimpleIntegerDiagnosticData lastError_; +}; + +} // namespace particle diff --git a/system/src/system_network_internal.cpp b/system/src/system_network_internal.cpp index 68023af2e3..a1e19f3ea7 100644 --- a/system/src/system_network_internal.cpp +++ b/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 */ diff --git a/system/src/system_network_internal.h b/system/src/system_network_internal.h index 0a09d942db..795d8d5d0a 100644 --- a/system/src/system_network_internal.h +++ b/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_; - AtomicEnumDiagnosticData 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. diff --git a/system/src/system_network_manager.cpp b/system/src/system_network_manager.cpp index be01a639a2..5a9a44ba1e 100644 --- a/system/src/system_network_manager.cpp +++ b/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); diff --git a/system/src/system_network_manager.h b/system/src/system_network_manager.h index 78fa182930..63b7da0933 100644 --- a/system/src/system_network_manager.h +++ b/system/src/system_network_manager.h @@ -26,6 +26,7 @@ #include "resolvapi.h" #include #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(); diff --git a/system/src/system_network_manager_api.cpp b/system/src/system_network_manager_api.cpp index 0b1306b4de..b9a75c3ec5 100644 --- a/system/src/system_network_manager_api.cpp +++ b/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);