Cleanup

src/SafeFile.cpp

@@ -11,12 +11,18 @@ static File openFile(const char *filename, bool fullAtomic)
     FSCom.remove(filename);
     return FSCom.open(filename, FILE_O_WRITE);
 #endif
-    if (!fullAtomic)
+    if (!fullAtomic) {
         FSCom.remove(filename); // Nuke the old file to make space (ignore if it !exists)
+    }
 
     String filenameTmp = filename;
     filenameTmp += ".tmp";
 
+    // FIXME: If we are doing a full atomic write, we may need to remove the old tmp file now
+    // if (fullAtomic) {
+    //     FSCom.remove(filename);
+    // }
+
     // clear any previous LFS errors
     return FSCom.open(filenameTmp.c_str(), FILE_O_WRITE);
 }

src/mesh/MeshService.cpp

@@ -125,17 +125,15 @@ void MeshService::loop()
 }
 
 /// The radioConfig object just changed, call this to force the hw to change to the new settings
-bool MeshService::reloadConfig(int saveWhat)
+void MeshService::reloadConfig(int saveWhat)
 {
     // If we can successfully set this radio to these settings, save them to disk
 
     // This will also update the region as needed
-    bool didReset = nodeDB->resetRadioConfig(); // Don't let the phone send us fatally bad settings
+    nodeDB->resetRadioConfig(); // Don't let the phone send us fatally bad settings
 
     configChanged.notifyObservers(NULL); // This will cause radio hardware to change freqs etc
     nodeDB->saveToDisk(saveWhat);
-
-    return didReset;
 }
 
 /// The owner User record just got updated, update our node DB and broadcast the info into the mesh

src/mesh/MeshService.h

@@ -118,7 +118,7 @@ class MeshService
     /** The radioConfig object just changed, call this to force the hw to change to the new settings
      * @return true if client devices should be sent a new set of radio configs
      */
-    bool reloadConfig(int saveWhat = SEGMENT_CONFIG | SEGMENT_MODULECONFIG | SEGMENT_DEVICESTATE | SEGMENT_CHANNELS);
+    void reloadConfig(int saveWhat = SEGMENT_CONFIG | SEGMENT_MODULECONFIG | SEGMENT_DEVICESTATE | SEGMENT_CHANNELS);
 
     /// The owner User record just got updated, update our node DB and broadcast the info into the mesh
     void reloadOwner(bool shouldSave = true);

src/mesh/NodeDB.cpp

@@ -400,18 +400,12 @@ bool isBroadcast(uint32_t dest)
     return dest == NODENUM_BROADCAST || dest == NODENUM_BROADCAST_NO_LORA;
 }
 
-bool NodeDB::resetRadioConfig(bool factory_reset, bool is_fresh_install)
+void NodeDB::resetRadioConfig(bool is_fresh_install)
 {
-    bool didFactoryReset = false;
-
     if (is_fresh_install) {
         radioGeneration++;
     }
 
-    if (factory_reset) {
-        didFactoryReset = factoryReset();
-    }
-
     if (channelFile.channels_count != MAX_NUM_CHANNELS) {
         LOG_INFO("Set default channel and radio preferences!");
 
@@ -422,14 +416,6 @@ bool NodeDB::resetRadioConfig(bool factory_reset, bool is_fresh_install)
 
     // Update the global myRegion
     initRegion();
-
-    if (didFactoryReset) {
-        LOG_INFO("Reboot due to factory reset");
-        screen->startAlert("Rebooting...");
-        rebootAtMsec = millis() + (5 * 1000);
-    }
-
-    return didFactoryReset;
 }
 
 bool NodeDB::factoryReset(bool eraseBleBonds)
@@ -591,7 +577,7 @@ void NodeDB::installDefaultConfig(bool preserveKey = false)
         config.device.node_info_broadcast_secs = default_node_info_broadcast_secs;
     config.security.serial_enabled = true;
     config.security.admin_channel_enabled = false;
-    resetRadioConfig(false, true); // This also triggers NodeInfo/Position requests since we're fresh
+    resetRadioConfig(true); // This also triggers NodeInfo/Position requests since we're fresh
     strncpy(config.network.ntp_server, "meshtastic.pool.ntp.org", 32);
 
 #if (defined(T_DECK) || defined(T_WATCH_S3) || defined(UNPHONE) || defined(PICOMPUTER_S3) || defined(SENSECAP_INDICATOR)) &&     \

src/mesh/NodeDB.h

@@ -103,7 +103,7 @@ class NodeDB
      * @param is_fresh_install set to true after a fresh install, to trigger NodeInfo/Position requests
      * @return true if the config was completely reset, in that case, we should send it back to the client
      */
-    bool resetRadioConfig(bool factory_reset = false, bool is_fresh_install = false);
+    void resetRadioConfig(bool is_fresh_install = false);
 
     /// given a subpacket sniffed from the network, update our DB state
     /// we updateGUI and updateGUIforNode if we think our this change is big enough for a redraw

src/mesh/RadioInterface.cpp

@@ -656,7 +656,7 @@ size_t RadioInterface::beginSending(meshtastic_MeshPacket *p)
 
     // if the sender nodenum is zero, that means uninitialized
     assert(radioBuffer.header.from);
-
+    assert(p->encrypted.size <= sizeof(radioBuffer.payload));
     memcpy(radioBuffer.payload, p->encrypted.bytes, p->encrypted.size);
 
     sendingPacket = p;

src/mesh/Router.cpp

@@ -198,6 +198,14 @@ ErrorCode Router::sendLocal(meshtastic_MeshPacket *p, RxSource src)
         return send(p);
     }
 }
+/**
+ * Send a packet on a suitable interface.
+ */
+ErrorCode Router::rawSend(meshtastic_MeshPacket *p)
+{
+    assert(iface); // This should have been detected already in sendLocal (or we just received a packet from outside)
+    return iface->send(p);
+}
 
 /**
  * Send a packet on a suitable interface.  This routine will
@@ -319,27 +327,27 @@ void Router::sniffReceived(const meshtastic_MeshPacket *p, const meshtastic_Rout
     // FIXME, update nodedb here for any packet that passes through us
 }
 
-bool perhapsDecode(meshtastic_MeshPacket *p)
+DecodeState perhapsDecode(meshtastic_MeshPacket *p)
 {
     concurrency::LockGuard g(cryptLock);
 
     if (config.device.role == meshtastic_Config_DeviceConfig_Role_REPEATER &&
         config.device.rebroadcast_mode == meshtastic_Config_DeviceConfig_RebroadcastMode_ALL_SKIP_DECODING)
-        return false;
+        return DecodeState::DECODE_FAILURE;
 
     if (config.device.rebroadcast_mode == meshtastic_Config_DeviceConfig_RebroadcastMode_KNOWN_ONLY &&
         (nodeDB->getMeshNode(p->from) == NULL || !nodeDB->getMeshNode(p->from)->has_user)) {
         LOG_DEBUG("Node 0x%x not in nodeDB-> Rebroadcast mode KNOWN_ONLY will ignore packet", p->from);
-        return false;
+        return DecodeState::DECODE_FAILURE;
     }
 
     if (p->which_payload_variant == meshtastic_MeshPacket_decoded_tag)
-        return true; // If packet was already decoded just return
+        return DecodeState::DECODE_SUCCESS; // If packet was already decoded just return
 
     size_t rawSize = p->encrypted.size;
     if (rawSize > sizeof(bytes)) {
         LOG_ERROR("Packet too large to attempt decryption! (rawSize=%d > 256)", rawSize);
-        return false;
+        return DecodeState::DECODE_FATAL;
     }
     bool decrypted = false;
     ChannelIndex chIndex = 0;
@@ -353,18 +361,22 @@ bool perhapsDecode(meshtastic_MeshPacket *p)
         if (crypto->decryptCurve25519(p->from, nodeDB->getMeshNode(p->from)->user.public_key, p->id, rawSize, p->encrypted.bytes,
                                       bytes)) {
             LOG_INFO("PKI Decryption worked!");
-            memset(&p->decoded, 0, sizeof(p->decoded));
+
+            meshtastic_Data decodedtmp;
+            memset(&decodedtmp, 0, sizeof(decodedtmp));
             rawSize -= MESHTASTIC_PKC_OVERHEAD;
-            if (pb_decode_from_bytes(bytes, rawSize, &meshtastic_Data_msg, &p->decoded) &&
+            if (pb_decode_from_bytes(bytes, rawSize, &meshtastic_Data_msg, &decodedtmp) &&
-                p->decoded.portnum != meshtastic_PortNum_UNKNOWN_APP) {
+                decodedtmp.portnum != meshtastic_PortNum_UNKNOWN_APP) {
                 decrypted = true;
                 LOG_INFO("Packet decrypted using PKI!");
                 p->pki_encrypted = true;
                 memcpy(&p->public_key.bytes, nodeDB->getMeshNode(p->from)->user.public_key.bytes, 32);
                 p->public_key.size = 32;
+                memcpy(&p->decoded, &decodedtmp, sizeof(meshtastic_Data_msg));
+                p->which_payload_variant = meshtastic_MeshPacket_decoded_tag; // change type to decoded
             } else {
                 LOG_ERROR("PKC Decrypted, but pb_decode failed!");
-                return false;
+                return DecodeState::DECODE_FAILURE;
             }
         } else {
             LOG_WARN("PKC decrypt attempted but failed!");
@@ -387,12 +399,15 @@ bool perhapsDecode(meshtastic_MeshPacket *p)
                 // printBytes("plaintext", bytes, p->encrypted.size);
 
                 // Take those raw bytes and convert them back into a well structured protobuf we can understand
-                memset(&p->decoded, 0, sizeof(p->decoded));
+                meshtastic_Data decodedtmp;
-                if (!pb_decode_from_bytes(bytes, rawSize, &meshtastic_Data_msg, &p->decoded)) {
+                memset(&decodedtmp, 0, sizeof(decodedtmp));
+                if (!pb_decode_from_bytes(bytes, rawSize, &meshtastic_Data_msg, &decodedtmp)) {
                     LOG_ERROR("Invalid protobufs in received mesh packet id=0x%08x (bad psk?)!", p->id);
-                } else if (p->decoded.portnum == meshtastic_PortNum_UNKNOWN_APP) {
+                } else if (decodedtmp.portnum == meshtastic_PortNum_UNKNOWN_APP) {
                     LOG_ERROR("Invalid portnum (bad psk?)!");
                 } else {
+                    p->decoded = decodedtmp;
+                    p->which_payload_variant = meshtastic_MeshPacket_decoded_tag; // change type to decoded
                     decrypted = true;
                     break;
                 }
@@ -401,8 +416,7 @@ bool perhapsDecode(meshtastic_MeshPacket *p)
     }
     if (decrypted) {
         // parsing was successful
-        p->which_payload_variant = meshtastic_MeshPacket_decoded_tag; // change type to decoded
+        p->channel = chIndex; // change to store the index instead of the hash
-        p->channel = chIndex;                                         // change to store the index instead of the hash
         if (p->decoded.has_bitfield)
             p->decoded.want_response |= p->decoded.bitfield & BITFIELD_WANT_RESPONSE_MASK;
 
@@ -434,10 +448,10 @@ bool perhapsDecode(meshtastic_MeshPacket *p)
             LOG_TRACE("%s", MeshPacketSerializer::JsonSerialize(p, false).c_str());
         }
 #endif
-        return true;
+        return DecodeState::DECODE_SUCCESS;
     } else {
         LOG_WARN("No suitable channel found for decoding, hash was 0x%x!", p->channel);
-        return false;
+        return DecodeState::DECODE_FAILURE;
     }
 }
 
@@ -592,8 +606,13 @@ void Router::handleReceived(meshtastic_MeshPacket *p, RxSource src)
     meshtastic_MeshPacket *p_encrypted = packetPool.allocCopy(*p);
 
     // Take those raw bytes and convert them back into a well structured protobuf we can understand
-    bool decoded = perhapsDecode(p);
+    auto decodedState = perhapsDecode(p);
-    if (decoded) {
+    if (decodedState == DecodeState::DECODE_FATAL) {
+        // Fatal decoding error, we can't do anything with this packet
+        LOG_WARN("Fatal decode error, dropping packet");
+        cancelSending(p->from, p->id);
+        skipHandle = true;
+    } else if (decodedState == DecodeState::DECODE_SUCCESS) {
         // parsing was successful, queue for our recipient
         if (src == RX_SRC_LOCAL)
             printPacket("handleReceived(LOCAL)", p);
@@ -636,10 +655,12 @@ void Router::handleReceived(meshtastic_MeshPacket *p, RxSource src)
 #if !MESHTASTIC_EXCLUDE_MQTT
         // Mark as pki_encrypted if it is not yet decoded and MQTT encryption is also enabled, hash matches and it's a DM not to
         // us (because we would be able to decrypt it)
-        if (!decoded && moduleConfig.mqtt.encryption_enabled && p->channel == 0x00 && !isBroadcast(p->to) && !isToUs(p))
+        if (decodedState == DecodeState::DECODE_FAILURE && moduleConfig.mqtt.encryption_enabled && p->channel == 0x00 &&
+            !isBroadcast(p->to) && !isToUs(p))
             p_encrypted->pki_encrypted = true;
         // After potentially altering it, publish received message to MQTT if we're not the original transmitter of the packet
-        if ((decoded || p_encrypted->pki_encrypted) && moduleConfig.mqtt.enabled && !isFromUs(p) && mqtt)
+        if ((decodedState == DecodeState::DECODE_SUCCESS || p_encrypted->pki_encrypted) && moduleConfig.mqtt.enabled &&
+            !isFromUs(p) && mqtt)
             mqtt->onSend(*p_encrypted, *p, p->channel);
 #endif
     }

src/mesh/Router.h

@@ -85,6 +85,7 @@ class Router : protected concurrency::OSThread, protected PacketHistory
      * NOTE: This method will free the provided packet (even if we return an error code)
      */
     virtual ErrorCode send(meshtastic_MeshPacket *p);
+    virtual ErrorCode rawSend(meshtastic_MeshPacket *p);
 
     /* Statistics for the amount of duplicate received packets and the amount of times we cancel a relay because someone did it
         before us */
@@ -139,12 +140,14 @@ class Router : protected concurrency::OSThread, protected PacketHistory
     void abortSendAndNak(meshtastic_Routing_Error err, meshtastic_MeshPacket *p);
 };
 
+enum DecodeState { DECODE_SUCCESS, DECODE_FAILURE, DECODE_FATAL };
+
 /** FIXME - move this into a mesh packet class
  * Remove any encryption and decode the protobufs inside this packet (if necessary).
  *
  * @return true for success, false for corrupt packet.
  */
-bool perhapsDecode(meshtastic_MeshPacket *p);
+DecodeState perhapsDecode(meshtastic_MeshPacket *p);
 
 /** Return 0 for success or a Routing_Error code for failure
  */

src/modules/RoutingModule.cpp

@@ -46,11 +46,6 @@ meshtastic_MeshPacket *RoutingModule::allocReply()
         return NULL;
     assert(currentRequest);
 
-    // We only consider making replies if the request was a legit routing packet (not just something we were sniffing)
-    if (currentRequest->decoded.portnum == meshtastic_PortNum_ROUTING_APP) {
-        assert(0); // 1.2 refactoring fixme, Not sure if anything needs this yet?
-        // return allocDataProtobuf(u);
-    }
     return NULL;
 }
 

src/mqtt/MQTT.cpp

@@ -117,7 +117,8 @@ inline void onReceiveProto(char *topic, byte *payload, size_t length)
         // likely they discovered each other via a channel we have downlink enabled for
         if (isToUs(p.get()) || (tx && tx->has_user && rx && rx->has_user))
             router->enqueueReceivedMessage(p.release());
-    } else if (router && perhapsDecode(p.get())) // ignore messages if we don't have the channel key
+    } else if (router &&
+               perhapsDecode(p.get()) == DecodeState::DECODE_SUCCESS) // ignore messages if we don't have the channel key
         router->enqueueReceivedMessage(p.release());
 }