diff --git a/src/openlcb/If.hxx b/src/openlcb/If.hxx index f76384c2c..95364c8d0 100644 --- a/src/openlcb/If.hxx +++ b/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. diff --git a/src/openlcb/IfCan.cxx b/src/openlcb/IfCan.cxx index 704b16f23..6f352af1e 100644 --- a/src/openlcb/IfCan.cxx +++ b/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 diff --git a/src/openlcb/IfCan.cxxtest b/src/openlcb/IfCan.cxxtest index 5a7c267b0..978e03f75 100644 --- a/src/openlcb/IfCan.cxxtest +++ b/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()); diff --git a/src/openlcb/IfCan.hxx b/src/openlcb/IfCan.hxx index b5121483f..442179d96 100644 --- a/src/openlcb/IfCan.hxx +++ b/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; diff --git a/src/openlcb/IfTcp.cxx b/src/openlcb/IfTcp.cxx index df91922f5..5a659cc3e 100644 --- a/src/openlcb/IfTcp.cxx +++ b/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 diff --git a/src/openlcb/IfTcp.cxxtest b/src/openlcb/IfTcp.cxxtest index 08431a20b..cb70dd472 100644 --- a/src/openlcb/IfTcp.cxxtest +++ b/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(); diff --git a/src/openlcb/IfTcp.hxx b/src/openlcb/IfTcp.hxx index 96abaecef..3bdb3f5f9 100644 --- a/src/openlcb/IfTcp.hxx +++ b/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_; diff --git a/src/openlcb/IfTcpImpl.hxx b/src/openlcb/IfTcpImpl.hxx index fb26358c0..da95b5042 100644 --- a/src/openlcb/IfTcpImpl.hxx +++ b/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 diff --git a/src/openlcb/MemoryConfig.cxx b/src/openlcb/MemoryConfig.cxx index f6d6ee215..cd4f0a843 100644 --- a/src/openlcb/MemoryConfig.cxx +++ b/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::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::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) diff --git a/src/openlcb/MemoryConfig.cxxtest b/src/openlcb/MemoryConfig.cxxtest index 839a3115c..b23251a0b 100644 --- a/src/openlcb/MemoryConfig.cxxtest +++ b/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,6 +379,7 @@ TEST_F(MemoryConfigTest, GetSpaceInfoRO_NZLA) struct GlobalMock : public Singleton { MOCK_METHOD0(reboot, void()); MOCK_METHOD0(factory_reset, void()); + MOCK_METHOD1(custom_factory_reset, void(NodeID)); }; extern "C" void reboot() @@ -385,6 +387,13 @@ 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 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 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 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 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 diff --git a/src/openlcb/MemoryConfig.hxx b/src/openlcb/MemoryConfig.hxx index 218a7b151..a6d049b22 100644 --- a/src/openlcb/MemoryConfig.hxx +++ b/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()) diff --git a/src/openlcb/MemoryConfigClient.cxxtest b/src/openlcb/MemoryConfigClient.cxxtest index c9e7f64a7..05afc63f0 100644 --- a/src/openlcb/MemoryConfigClient.cxxtest +++ b/src/openlcb/MemoryConfigClient.cxxtest @@ -33,7 +33,10 @@ */ #include "openlcb/MemoryConfigClient.hxx" + +#include "openlcb/ConfigUpdateFlow.hxx" #include "openlcb/DatagramCan.hxx" +#include "utils/ConfigUpdateListener.hxx" #include "utils/async_datagram_test_helper.hxx" @@ -98,6 +101,32 @@ public: SyncNotifiable n_; }; +struct GlobalMock : public Singleton +{ + MOCK_METHOD0(reboot, void()); + MOCK_METHOD0(factory_reset, void()); +}; + +extern "C" void reboot() +{ + GlobalMock::instance()->reboot(); +} + +struct FactoryResetListener : public DefaultConfigUpdateListener +{ + void factory_reset(int fd) override + { + GlobalMock::instance()->factory_reset(); + } + + UpdateAction apply_configuration( + int fd, bool initial_load, BarrierNotifiable *done) + { + done->notify(); + return UPDATED; + } +}; + TEST_F(MemoryConfigClientTest, create) { } @@ -216,7 +245,7 @@ TEST_F(MemoryConfigClientTest, reboot_reboot) EXPECT_EQ(MemoryConfigClient::OPERATION_PENDING, b->data()->resultCode); send_packet(":X19100499N050101011400;"); wait(); // we purposefully do not do timed wait here; the init complete - // message sould terminate the timeout timer. + // message should terminate the timeout timer. EXPECT_EQ(0, b->data()->resultCode); ASSERT_TRUE(b->data()->done.is_done()); } @@ -260,6 +289,131 @@ TEST_F(MemoryConfigClientTest, unfreeze) ASSERT_TRUE(b->data()->done.is_done()); } +class FactoryResetTest : public MemoryConfigClientTest +{ +protected: + FactoryResetTest() + { + updateFlow_.TEST_set_fd(23); + eb_.release_block(); + twait(); + } + + ~FactoryResetTest() + { + twait(); + } + + StrictMock mock_; + BlockExecutor eb_ {ifCan_->executor()}; + ConfigUpdateFlow updateFlow_ {ifCan_.get()}; + FactoryResetListener l_; +}; // FactoryResetTest + +TEST_F(FactoryResetTest, create) +{ +} + +// Sends a factory reset command to a target represented with a node ID. +TEST_F(FactoryResetTest, send_with_id) +{ + print_all_packets(); + expect_any_packet(); + ::testing::InSequence seq; + EXPECT_CALL(mock_, factory_reset()); + EXPECT_CALL(mock_, reboot()); + auto b = invoke_flow(&clientTwo_, MemoryConfigClientRequest::FACTORY_RESET, + NodeHandle(TEST_NODE_ID)); + EXPECT_EQ(0, b->data()->resultCode); +} + +// Sends a factory reset command to a target represented with a node alias +// only, no ID. +TEST_F(FactoryResetTest, send_with_alias) +{ + print_all_packets(); + expect_any_packet(); + ::testing::InSequence seq; + EXPECT_CALL(mock_, factory_reset()); + EXPECT_CALL(mock_, reboot()); + auto b = invoke_flow(&clientTwo_, MemoryConfigClientRequest::FACTORY_RESET, + NodeHandle(NodeAlias(0x22A))); + EXPECT_EQ(0, b->data()->resultCode); +} + +// Sends a factory reset command to a the local node. +TEST_F(FactoryResetTest, do_local) +{ + print_all_packets(); + expect_any_packet(); + ::testing::InSequence seq; + EXPECT_CALL(mock_, factory_reset()); + EXPECT_CALL(mock_, reboot()); + auto b = invoke_flow(&clientTwo_, MemoryConfigClientRequest::FACTORY_RESET, + NodeHandle(TWO_NODE_ID)); + EXPECT_EQ(0, b->data()->resultCode); +} + +// Sends a factory reset command to a remote node by alias, with expectations +// on each packet. +TEST_F(FactoryResetTest, send_alias_lookup) +{ + print_all_packets(); + ::testing::InSequence seq; + expect_packet(":X19488FF2N0499;"); // looking for dstThree_ node + auto b = invoke_client_no_block( + MemoryConfigClientRequest::FACTORY_RESET, dstThree_); + wait(); + clear_expect(true); + EXPECT_EQ(MemoryConfigClient::OPERATION_PENDING, b->data()->resultCode); + + send_packet_and_expect_response( // + ":X19170499N010203040506;", // + ":X1A499FF2N20AA010203040506;"); + clear_expect(true); + EXPECT_EQ(MemoryConfigClient::OPERATION_PENDING, b->data()->resultCode); + + // Reject with an error. + send_packet(":X19A48499N0FF21234;"); + twait(); + + EXPECT_EQ(0x1234, b->data()->resultCode); + + EXPECT_CALL(mock_, factory_reset()).Times(0); + EXPECT_CALL(mock_, reboot()).Times(0); +} + +// Sends a factory reset command to a remote node by alias, when there is +// already a cache entry for that remote alias. +TEST_F(FactoryResetTest, cached_alias) +{ + print_all_packets(); + ::testing::InSequence seq; + // AMD frame to prefill cache. + send_packet(":X10701499N010203040506;"); + wait(); + // Datagram sent straight away + expect_packet(":X1A499FF2N20AA010203040506;"); + auto b = invoke_client_no_block( + MemoryConfigClientRequest::FACTORY_RESET, dstThree_); + wait(); + clear_expect(true); + EXPECT_EQ(MemoryConfigClient::OPERATION_PENDING, b->data()->resultCode); + + // Accept with no response pending + send_packet(":X19A28499N0FF200;"); + twait(); + + EXPECT_EQ(0, b->data()->resultCode); + + EXPECT_CALL(mock_, factory_reset()).Times(0); + EXPECT_CALL(mock_, reboot()).Times(0); +} + +TEST_F(MemoryConfigClientTest, factory_reset) +{ +} + class MemoryConfigLocalClientTest : public AsyncDatagramTest { protected: ~MemoryConfigLocalClientTest() { diff --git a/src/openlcb/MemoryConfigClient.hxx b/src/openlcb/MemoryConfigClient.hxx index 965400d3d..1ce044017 100644 --- a/src/openlcb/MemoryConfigClient.hxx +++ b/src/openlcb/MemoryConfigClient.hxx @@ -36,8 +36,9 @@ #define _OPENLCB_MEMORYCONFIGCLIENT_HXX_ #include "executor/CallableFlow.hxx" -#include "openlcb/MemoryConfig.hxx" #include "openlcb/DatagramHandlerDefault.hxx" +#include "openlcb/IfCan.hxx" +#include "openlcb/MemoryConfig.hxx" namespace openlcb { @@ -69,6 +70,11 @@ struct MemoryConfigClientRequest : public CallableFlowRequestBase REBOOT }; + enum FactoryResetCmd + { + FACTORY_RESET + }; + enum FreezeCmd { FREEZE @@ -164,6 +170,21 @@ struct MemoryConfigClientRequest : public CallableFlowRequestBase payload.push_back(MemoryConfigDefs::COMMAND_RESET); } + /// Sets up a command to send a Factory Reset request to a remote node. + /// @param FactoryResetCmd polymorphic matching arg; always set to + /// FACTORY_RESET. + /// @param d is the destination node + void reset(FactoryResetCmd, NodeHandle d) + { + reset_base(); + cmd = CMD_FACTORY_RESET; + dst = d; + payload.clear(); + payload.reserve(8); + payload.push_back(DatagramDefs::CONFIGURATION); + payload.push_back(MemoryConfigDefs::COMMAND_FACTORY_RESET); + } + /// Sets up a command to send a Freeze request to a remote node. /// @param FreezeCmd polymorphic matching arg; always set to /// FREEZE. @@ -203,7 +224,8 @@ struct MemoryConfigClientRequest : public CallableFlowRequestBase CMD_READ, CMD_READ_PART, CMD_WRITE, - CMD_META_REQUEST + CMD_META_REQUEST, + CMD_FACTORY_RESET }; /// Helper function invoked at every other reset call. @@ -277,6 +299,8 @@ private: case MemoryConfigClientRequest::CMD_META_REQUEST: return allocate_and_call( STATE(do_meta_request), dg_service()->client_allocator()); + case MemoryConfigClientRequest::CMD_FACTORY_RESET: + return call_immediately(STATE(prepare_factory_reset)); default: break; } @@ -580,6 +604,51 @@ private: } } + /// Before we send out a factory reset command, we have to ensure that we + /// know the target node's node ID, not just the alias. + Action prepare_factory_reset() + { + node()->iface()->canonicalize_handle(&request()->dst); + if (request()->dst.id) + { + return call_immediately(STATE(factory_reset_have_id)); + } + // Now: we have a dst with alias only, so we must be running on CAN-bus. + IfCan *iface = (IfCan *)node()->iface(); + if (!nodeIdlookupFlow_) + { + // This is so rarely used that we rather allocate it dynamically. + nodeIdlookupFlow_.reset(new NodeIdLookupFlow(iface)); + } + return invoke_subflow_and_wait(nodeIdlookupFlow_.get(), + STATE(dst_id_complete), node(), request()->dst); + } + + /// Completed the ID lookup flow. + Action dst_id_complete() + { + auto rb = + get_buffer_deleter(full_allocation_result(nodeIdlookupFlow_.get())); + request()->dst = rb->data()->handle; + // Object not needed anymore. + nodeIdlookupFlow_.reset(); + if (request()->dst.id) + { + return call_immediately(STATE(factory_reset_have_id)); + } + return return_with_error(Defs::ERROR_OPENMRN_NOT_FOUND); + } + + /// Called from different places to do the factory reset request once we + /// have the node ID filled in the dst handle. + Action factory_reset_have_id() + { + request()->payload.resize(8); + node_id_to_data(request()->dst.id, &request()->payload[2]); + return allocate_and_call( + STATE(do_meta_request), dg_service()->client_allocator()); + } + class ResponseFlow : public DefaultDatagramHandler { public: @@ -661,6 +730,8 @@ private: ResponseFlow responseFlow_{this}; /// Notify helper. BarrierNotifiable bn_; + /// Rarely used helper flow to look up full node IDs from aliases. + std::unique_ptr nodeIdlookupFlow_; /// Next byte to read from the memory space. uint32_t offset_; /// Next byte in the payload to write. diff --git a/src/utils/async_if_test_helper.hxx b/src/utils/async_if_test_helper.hxx index ea6e5cd11..473f379ec 100644 --- a/src/utils/async_if_test_helper.hxx +++ b/src/utils/async_if_test_helper.hxx @@ -325,8 +325,9 @@ static const NodeID TEST_NODE_ID = 0x02010d000003ULL; class LocalIf : public If { public: - LocalIf(int local_nodes_count) + LocalIf(int local_nodes_count, NodeID gateway_node_id) : If(&g_executor, local_nodes_count) + , gatewayNodeID_(gateway_node_id) { } @@ -352,8 +353,14 @@ public: return false; } + NodeID get_default_node_id() override + { + return gatewayNodeID_; + } + private: std::vector> ownedFlows_; + NodeID gatewayNodeID_; }; /** Test fixture base class with helper methods for exercising the asynchronous