Fix: return failure when PhoneAPI times out (#3136)

* Add debug options for RP2040

* Rename: "observed" should be plural: "observables"

* PhoneAPI: return failure on timeout
In `onNotify()`, when disconnected, PhoneAPI removed itself from the list of observers that was looped through in `notifyObservers()`. We should exit that loop in that case.
This commit is contained in:
GUVWAF 2024-01-28 14:53:39 +01:00 committed by GitHub
parent d604a76c73
commit 417feee47f
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 28 additions and 19 deletions

View File

@ -10,12 +10,12 @@ template <class T> class Observable;
*/ */
template <class T> class Observer template <class T> class Observer
{ {
std::list<Observable<T> *> observed; std::list<Observable<T> *> observables;
public: public:
virtual ~Observer(); virtual ~Observer();
/// Stop watching the obserable /// Stop watching the observable
void unobserve(Observable<T> *o); void unobserve(Observable<T> *o);
/// Start watching a specified observable /// Start watching a specified observable
@ -86,21 +86,21 @@ template <class T> class Observable
template <class T> Observer<T>::~Observer() template <class T> Observer<T>::~Observer()
{ {
for (typename std::list<Observable<T> *>::const_iterator iterator = observed.begin(); iterator != observed.end(); for (typename std::list<Observable<T> *>::const_iterator iterator = observables.begin(); iterator != observables.end();
++iterator) { ++iterator) {
(*iterator)->removeObserver(this); (*iterator)->removeObserver(this);
} }
observed.clear(); observables.clear();
} }
template <class T> void Observer<T>::unobserve(Observable<T> *o) template <class T> void Observer<T>::unobserve(Observable<T> *o)
{ {
o->removeObserver(this); o->removeObserver(this);
observed.remove(o); observables.remove(o);
} }
template <class T> void Observer<T>::observe(Observable<T> *o) template <class T> void Observer<T>::observe(Observable<T> *o)
{ {
observed.push_back(o); observables.push_back(o);
o->addObserver(this); o->addObserver(this);
} }

View File

@ -108,7 +108,8 @@ void MeshService::loop()
(void)sendQueueStatusToPhone(qs, 0, 0); (void)sendQueueStatusToPhone(qs, 0, 0);
} }
if (oldFromNum != fromNum) { // We don't want to generate extra notifies for multiple new packets if (oldFromNum != fromNum) { // We don't want to generate extra notifies for multiple new packets
fromNumChanged.notifyObservers(fromNum); int result = fromNumChanged.notifyObservers(fromNum);
if (result == 0) // If any observer returns non-zero, we will try again
oldFromNum = fromNum; oldFromNum = fromNum;
} }
} }

View File

@ -62,15 +62,17 @@ void PhoneAPI::close()
} }
} }
void PhoneAPI::checkConnectionTimeout() bool PhoneAPI::checkConnectionTimeout()
{ {
if (isConnected()) { if (isConnected()) {
bool newContact = checkIsConnected(); bool newContact = checkIsConnected();
if (!newContact) { if (!newContact) {
LOG_INFO("Lost phone connection\n"); LOG_INFO("Lost phone connection\n");
close(); close();
return true;
} }
} }
return false;
} }
/** /**
@ -461,8 +463,8 @@ bool PhoneAPI::handleToRadioPacket(meshtastic_MeshPacket &p)
/// If the mesh service tells us fromNum has changed, tell the phone /// If the mesh service tells us fromNum has changed, tell the phone
int PhoneAPI::onNotify(uint32_t newValue) int PhoneAPI::onNotify(uint32_t newValue)
{ {
checkConnectionTimeout(); // a handy place to check if we've heard from the phone (since the BLE version doesn't call this bool timeout = checkConnectionTimeout(); // a handy place to check if we've heard from the phone (since the BLE version
// from idle) // doesn't call this from idle)
if (state == STATE_SEND_PACKETS) { if (state == STATE_SEND_PACKETS) {
LOG_INFO("Telling client we have new packets %u\n", newValue); LOG_INFO("Telling client we have new packets %u\n", newValue);
@ -471,5 +473,5 @@ int PhoneAPI::onNotify(uint32_t newValue)
LOG_DEBUG("(Client not yet interested in packets)\n"); LOG_DEBUG("(Client not yet interested in packets)\n");
} }
return 0; return timeout ? -1 : 0; // If we timed out, MeshService should stop iterating through observers as we just removed one
} }

View File

@ -108,8 +108,8 @@ class PhoneAPI
/// Hookable to find out when connection changes /// Hookable to find out when connection changes
virtual void onConnectionChanged(bool connected) {} virtual void onConnectionChanged(bool connected) {}
/// If we haven't heard from the other side in a while then say not connected /// If we haven't heard from the other side in a while then say not connected. Returns true if timeout occurred
void checkConnectionTimeout(); bool checkConnectionTimeout();
/// Check the current underlying physical link to see if the client is currently connected /// Check the current underlying physical link to see if the client is currently connected
virtual bool checkIsConnected() = 0; virtual bool checkIsConnected() = 0;

View File

@ -11,3 +11,5 @@ build_flags = ${rp2040_base.build_flags}
-L "${platformio.libdeps_dir}/${this.__env__}/BSEC2 Software Library/src/cortex-m0plus" -L "${platformio.libdeps_dir}/${this.__env__}/BSEC2 Software Library/src/cortex-m0plus"
lib_deps = lib_deps =
${rp2040_base.lib_deps} ${rp2040_base.lib_deps}
debug_build_flags = ${rp2040_base.build_flags}
debug_tool = cmsis-dap ; for e.g. Picotool

View File

@ -12,3 +12,5 @@ build_flags = ${rp2040_base.build_flags}
-L "${platformio.libdeps_dir}/${this.__env__}/BSEC2 Software Library/src/cortex-m0plus" -L "${platformio.libdeps_dir}/${this.__env__}/BSEC2 Software Library/src/cortex-m0plus"
lib_deps = lib_deps =
${rp2040_base.lib_deps} ${rp2040_base.lib_deps}
debug_build_flags = ${rp2040_base.build_flags}
debug_tool = cmsis-dap ; for e.g. Picotool

View File

@ -15,3 +15,5 @@ build_src_filter = ${rp2040_base.build_src_filter} +<mesh/wifi/>
lib_deps = lib_deps =
${rp2040_base.lib_deps} ${rp2040_base.lib_deps}
${networking_base.lib_deps} ${networking_base.lib_deps}
debug_build_flags = ${rp2040_base.build_flags}
debug_tool = cmsis-dap ; for e.g. Picotool