Factory reset improvements

src/openlcb/If.hxx

@@ -325,6 +325,11 @@ public:
      * on the interface executor. */
     virtual void canonicalize_handle(NodeHandle *h) {}
 
+    /// @return the node ID of the default node on this interface. For TCP
+    /// interfaces this is the gateway node ID, for CAN interfaces this is the
+    /// node ID used for alias allocation on the CAN-bus.
+    virtual NodeID get_default_node_id() = 0;
+
     /// Sets a transmit hook. This function will be called once for every
     /// OpenLCB message transmitted. Used for implementing activity LEDs.
     /// @param hook function to call for each transmit message.

src/openlcb/IfCan.cxx

@@ -805,4 +805,13 @@ Node *IfCan::lookup_local_node_handle(NodeHandle h)
     return lookup_local_node(h.id);
 }
 
+NodeID IfCan::get_default_node_id()
+{
+    if (!aliasAllocator_)
+    {
+        return 0;
+    }
+    return aliasAllocator_->if_node_id();
+}
+
 } // namespace openlcb

src/openlcb/IfCan.cxxtest

@@ -141,6 +141,11 @@ TEST_F(AsyncIfTest, AMEReceiveSupport)
     wait();
 }
 
+TEST_F(AsyncIfTest, GetDefaultNodeId)
+{
+    EXPECT_EQ(TEST_NODE_ID, ifCan_->get_default_node_id());
+}
+
 TEST_F(AsyncNodeTest, GlobalAMESendSupport)
 {
     EXPECT_TRUE(node_->is_initialized());

src/openlcb/IfCan.hxx

@@ -132,6 +132,8 @@ public:
 
     Node *lookup_local_node_handle(NodeHandle handle) override;
 
+    NodeID get_default_node_id() override;
+
 private:
     void canonicalize_handle(NodeHandle *h) override;
 

src/openlcb/IfTcp.cxx

@@ -210,6 +210,11 @@ IfTcp::~IfTcp()
     }
 }
 
+NodeID IfTcp::get_default_node_id()
+{
+    return sendFlow_->get_gateway_node_id();
+}
+
 } // namespace openlcb
 
 #endif // OPENMRN_FEATURE_EXECUTOR_SELECT

src/openlcb/IfTcp.cxxtest

@@ -151,7 +151,7 @@ protected:
     HubPortInterface *fakeSource_ = (HubPortInterface *)123456;
     TestSequenceGenerator seq_;
     static constexpr uint64_t GW_NODE_ID = 0x101112131415ULL;
-    LocalIf localIf_{10};
+    LocalIf localIf_ {10, GW_NODE_ID};
     TcpSendFlow sendFlow_{
         &localIf_, GW_NODE_ID, &fakeSendTarget_, fakeSource_, &seq_};
 };
@@ -409,6 +409,11 @@ TEST_F(TcpIfTest, create)
 {
 }
 
+TEST_F(TcpIfTest, get_node_id)
+{
+    EXPECT_EQ(GW_NODE_ID, ifTcp_.get_default_node_id());
+}
+
 TEST_F(TcpIfTest, send_message_global)
 {
     capture_next_packet();

src/openlcb/IfTcp.hxx

@@ -113,6 +113,9 @@ public:
     /// @param on_error will be invoked when a socket error is encountered.
     void add_network_fd(int fd, Notifiable *on_error = nullptr);
 
+    /// @return the gateway node ID.
+    NodeID get_default_node_id() override;
+
 private:
     /// Where to send traffic to.
     HubFlow *device_;

src/openlcb/IfTcpImpl.hxx

@@ -303,6 +303,12 @@ public:
     {
     }
 
+    /// @return the node ID the gateway uses to set on outgoing messages.
+    NodeID get_gateway_node_id()
+    {
+        return gatewayId_;
+    }
+
 private:
     /// Handler where dequeueing of messages to be sent starts.
     /// @return next state

src/openlcb/MemoryConfig.cxx

@@ -73,13 +73,26 @@ static constexpr unsigned FACTORY_RESET_REBOOT_DELAY_MSEC = 50;
 static constexpr unsigned FACTORY_RESET_REBOOT_DELAY_MSEC = 500;
 #endif
 
+uint16_t __attribute__((weak, noinline))
+MemoryConfigHandler::app_handle_factory_reset(NodeID target)
+{
+    return Defs::ERROR_UNIMPLEMENTED;
+}
 
-void __attribute__((weak)) MemoryConfigHandler::handle_factory_reset()
+uint16_t MemoryConfigHandler::handle_factory_reset(NodeID target)
 {
-    static_cast<ConfigUpdateFlow *>(ConfigUpdateFlow::instance())
-        ->factory_reset();
-    (new RebootTimer(service()))
-        ->start(MSEC_TO_NSEC(FACTORY_RESET_REBOOT_DELAY_MSEC));
+    if (target == dg_service()->iface()->get_default_node_id())
+    {
+        static_cast<ConfigUpdateFlow *>(ConfigUpdateFlow::instance())
+            ->factory_reset();
+        (new RebootTimer(service()))
+            ->start(MSEC_TO_NSEC(FACTORY_RESET_REBOOT_DELAY_MSEC));
+        return 0;
+    }
+    else
+    {
+        return app_handle_factory_reset(target);
+    }
 }
 
 FileMemorySpace::FileMemorySpace(int fd, address_t len)

src/openlcb/MemoryConfig.cxxtest

@@ -64,7 +64,8 @@ public:
 class MemoryConfigTest : public TwoNodeDatagramTest
 {
 protected:
-    MemoryConfigTest() : memoryOne_(&datagram_support_, node_, 10)
+    MemoryConfigTest()
+        : memoryOne_(&datagram_support_, nullptr, 10)
     {
         ON_CALL(space, write(_, _, _, _, _))
             .WillByDefault(DoAll(SetArgPointee<3>(0x2F00), Return(0)));
@@ -378,13 +379,21 @@ TEST_F(MemoryConfigTest, GetSpaceInfoRO_NZLA)
 struct GlobalMock : public Singleton<GlobalMock> {
     MOCK_METHOD0(reboot, void());
     MOCK_METHOD0(factory_reset, void());
+    MOCK_METHOD1(custom_factory_reset, void(NodeID));
 };
 
 extern "C" void reboot()
 {
     GlobalMock::instance()->reboot();
 }
 
+// Overrides the weak implementation in the stack.
+uint16_t MemoryConfigHandler::app_handle_factory_reset(NodeID id)
+{
+    GlobalMock::instance()->custom_factory_reset(id);
+    return 0x1234;
+}
+
 struct FactoryResetListener : public DefaultConfigUpdateListener
 {
     void factory_reset(int fd) override
@@ -412,6 +421,42 @@ TEST_F(MemoryConfigTest, Reboot)
     wait();
 }
 
+TEST_F(MemoryConfigTest, FactoryResetWrongNodeId)
+{
+    StrictMock<GlobalMock> mock;
+    ConfigUpdateFlow update_flow {ifCan_.get()};
+    update_flow.TEST_set_fd(23);
+
+    FactoryResetListener l;
+    // rejected with error "invalid arguments"
+    expect_packet(":X19A4822AN077C1080;");
+
+    EXPECT_CALL(mock, factory_reset()).Times(0);
+    send_packet(":X1A22A77CN20AA010101010101;");
+    wait();
+
+    EXPECT_CALL(mock, reboot()).Times(0);
+    twait();
+}
+
+TEST_F(MemoryConfigTest, FactoryResetNoNodeId)
+{
+    StrictMock<GlobalMock> mock;
+    ConfigUpdateFlow update_flow {ifCan_.get()};
+    update_flow.TEST_set_fd(23);
+
+    FactoryResetListener l;
+    // rejected with error "invalid arguments"
+    expect_packet(":X19A4822AN077C1080;");
+
+    EXPECT_CALL(mock, factory_reset()).Times(0);
+    send_packet(":X1A22A77CN20AA;");
+    wait();
+
+    EXPECT_CALL(mock, reboot()).Times(0);
+    twait();
+}
+
 TEST_F(MemoryConfigTest, FactoryReset)
 {
     StrictMock<GlobalMock> mock;
@@ -422,13 +467,38 @@ TEST_F(MemoryConfigTest, FactoryReset)
     expect_packet(":X19A2822AN077C00;"); // received OK, no response
 
     EXPECT_CALL(mock, factory_reset());
-    send_packet(":X1A22A77CN20AA;");
+    send_packet(":X1A22A77CN20AA02010d000003;");
     wait();
 
     EXPECT_CALL(mock, reboot());
     twait();
 }
 
+// Tests addressing a factory reset command to a virtual node (non-default
+// node).
+TEST_F(MemoryConfigTest, FactoryResetVNode)
+{
+    setup_other_node(false);
+    StrictMock<GlobalMock> mock;
+    ConfigUpdateFlow update_flow {ifCan_.get()};
+    update_flow.TEST_set_fd(23);
+
+    FactoryResetListener l;
+
+    expect_packet(":X19A48225N077C1234;"); // Rejected with error 0x1234
+
+    // Default reset won't be invoked.
+    EXPECT_CALL(mock, factory_reset()).Times(0);
+    // Custom reset will.
+    EXPECT_CALL(mock, custom_factory_reset(OTHER_NODE_ID));
+    send_packet(":X1A22577CN20AA02010d000103;");
+    wait();
+
+    // No reboot.
+    EXPECT_CALL(mock, reboot()).Times(0);
+    twait();
+}
+
 static const char MEMORY_BLOCK_DATA[] = "abrakadabra12345678xxxxyyyyzzzzwww.";
 
 class StaticBlockTest : public MemoryConfigTest

src/openlcb/MemoryConfig.hxx

@@ -484,8 +484,20 @@ private:
             }
             case MemoryConfigDefs::COMMAND_FACTORY_RESET:
             {
-                handle_factory_reset();
-                return respond_ok(0);
+                NodeID id = message()->data()->dst->node_id();
+                if (len < 8 || (data_to_node_id(&bytes[2]) != id))
+                {
+                    return respond_reject(Defs::ERROR_INVALID_ARGS);
+                }
+                uint16_t ret = handle_factory_reset(id);
+                if (!ret)
+                {
+                    return respond_ok(0);
+                }
+                else
+                {
+                    return respond_reject(ret);
+                }
             }
             case MemoryConfigDefs::COMMAND_OPTIONS:
             {
@@ -536,8 +548,18 @@ private:
 
     /// Invokes the openlcb config handler to do a factory reset. Starts a
     /// timer to reboot the device after a little time.
-    void handle_factory_reset();
-
+    /// @param target the node ID for which factory reset was invoked.
+    /// @return openlcb error code, 0 on success
+    uint16_t handle_factory_reset(NodeID target);
+
+    /// Weak definition for invoking a factory reset on virtual nodes. The
+    /// default implementation does nothing and return an unimplemented
+    /// error. Applications that use virtual nodes and need to support factory
+    /// reset should reimplement this function.
+    /// @param target the node ID for which factory reset was invoked.
+    /// @return openlcb error code, 0 on success
+    uint16_t __attribute__((noinline)) app_handle_factory_reset(NodeID target);
+
     Action ok_response_sent() OVERRIDE
     {
         if (!response_.empty())