From 669028a567cfba869987df8fcc9eda015d5f8dc7 Mon Sep 17 00:00:00 2001 From: Adrian Del Grosso <10929341+ad3154@users.noreply.github.com> Date: Tue, 12 Mar 2024 19:26:18 -0600 Subject: [PATCH] [TC]: Add options for falling back to other language data sources when TC doesn't respond Allow the TC client to fall back to other language sources as needed. Fixed a condition where the TC client would become stuck while waiting for a language response if a response never arrived. Added a getter to the language command interface to allow retrieving the assigned partner CF. Prevent a crash in the language command interface which could occur if you send the command with some fields left unconfigured. --- .../isobus_language_command_interface.hpp | 4 + .../isobus/isobus_task_controller_client.hpp | 1 + .../src/isobus_language_command_interface.cpp | 10 +++ isobus/src/isobus_task_controller_client.cpp | 38 +++++++-- test/language_command_interface_tests.cpp | 3 + test/tc_client_tests.cpp | 85 ++++++++++++++++++- 6 files changed, 129 insertions(+), 12 deletions(-) diff --git a/isobus/include/isobus/isobus/isobus_language_command_interface.hpp b/isobus/include/isobus/isobus/isobus_language_command_interface.hpp index 4a84939cd..5dbe9cfee 100644 --- a/isobus/include/isobus/isobus/isobus_language_command_interface.hpp +++ b/isobus/include/isobus/isobus/isobus_language_command_interface.hpp @@ -166,6 +166,10 @@ namespace isobus /// @param[in] filteredControlFunction The new partner to communicate with void set_partner(std::shared_ptr filteredControlFunction); + /// @brief Returns the current partner being used by the interface + /// @return The current partner being used by the interface, or nullptr if none + std::shared_ptr get_partner() const; + /// @brief Returns if initialize has been called yet /// @return `true` if initialize has been called, otherwise false bool get_initialized() const; diff --git a/isobus/include/isobus/isobus/isobus_task_controller_client.hpp b/isobus/include/isobus/isobus/isobus_task_controller_client.hpp index 6a23073eb..ca82efd01 100644 --- a/isobus/include/isobus/isobus/isobus_task_controller_client.hpp +++ b/isobus/include/isobus/isobus/isobus_task_controller_client.hpp @@ -640,6 +640,7 @@ namespace isobus std::uint32_t statusMessageTimestamp_ms = 0; ///< Timestamp corresponding to the last time we sent a status message to the TC std::uint32_t serverStatusMessageTimestamp_ms = 0; ///< Timestamp corresponding to the last time we received a status message from the TC std::uint32_t userSuppliedBinaryDDOPSize_bytes = 0; ///< The number of bytes in the user provided binary DDOP (if one was provided) + std::uint32_t languageCommandWaitingTimestamp_ms = 0; ///< Timestamp used to determine when to give up on waiting for a language command response std::uint8_t numberOfWorkingSetMembers = 1; ///< The number of working set members that will be reported in the working set master message std::uint8_t tcStatusBitfield = 0; ///< The last received TC/DL status from the status message std::uint8_t sourceAddressOfCommandBeingExecuted = 0; ///< Source address of client for which the current command is being executed diff --git a/isobus/src/isobus_language_command_interface.cpp b/isobus/src/isobus_language_command_interface.cpp index ae1a1a8db..5964a8f99 100644 --- a/isobus/src/isobus_language_command_interface.cpp +++ b/isobus/src/isobus_language_command_interface.cpp @@ -77,6 +77,11 @@ namespace isobus myPartner = filteredControlFunction; } + std::shared_ptr LanguageCommandInterface::get_partner() const + { + return myPartner; + } + bool LanguageCommandInterface::get_initialized() const { return initialized; @@ -100,6 +105,11 @@ namespace isobus bool LanguageCommandInterface::send_language_command() const { + if ((languageCode.length() < 2) || (countryCode.length() < 2)) + { + LOG_ERROR("[VT/TC]: Language command interface is missing language or country code, and will not send a language command."); + return false; + } std::array buffer{ static_cast(languageCode[0]), static_cast(languageCode[1]), diff --git a/isobus/src/isobus_task_controller_client.cpp b/isobus/src/isobus_task_controller_client.cpp index 311ec2c97..3bcd86401 100644 --- a/isobus/src/isobus_task_controller_client.cpp +++ b/isobus/src/isobus_task_controller_client.cpp @@ -511,15 +511,13 @@ namespace isobus LOG_WARNING("[TC]: The TC is < version 4 but no VT was provided. Language data will be requested globally, which might not be ideal."); } - if ((serverVersion < static_cast(Version::SecondPublishedEdition)) && - (nullptr != primaryVirtualTerminal) && - (primaryVirtualTerminal->languageCommandInterface.send_request_language_command())) - { - set_state(StateMachineState::WaitForLanguageResponse); - } - else if (languageCommandInterface.send_request_language_command()) + if (((serverVersion < static_cast(Version::SecondPublishedEdition)) && + (nullptr != primaryVirtualTerminal) && + (primaryVirtualTerminal->languageCommandInterface.send_request_language_command())) || + (languageCommandInterface.send_request_language_command())) { set_state(StateMachineState::WaitForLanguageResponse); + languageCommandWaitingTimestamp_ms = SystemTiming::get_timestamp_ms(); } else if (SystemTiming::time_expired_ms(stateMachineTimestamp_ms, SIX_SECOND_TIMEOUT_MS)) { @@ -531,11 +529,35 @@ namespace isobus case StateMachineState::WaitForLanguageResponse: { - if ((SystemTiming::get_time_elapsed_ms(languageCommandInterface.get_language_command_timestamp()) < SIX_SECOND_TIMEOUT_MS) && + if ((SystemTiming::get_time_elapsed_ms(languageCommandInterface.get_language_command_timestamp()) < TWO_SECOND_TIMEOUT_MS) && ("" != languageCommandInterface.get_language_code())) { set_state(StateMachineState::ProcessDDOP); } + else if (SystemTiming::time_expired_ms(languageCommandWaitingTimestamp_ms, SIX_SECOND_TIMEOUT_MS)) + { + LOG_WARNING("[TC]: Timeout waiting for language response. Moving on to processing the DDOP anyways."); + set_state(StateMachineState::ProcessDDOP); + } + else if ((SystemTiming::time_expired_ms(stateMachineTimestamp_ms, TWO_SECOND_TIMEOUT_MS)) && + (nullptr != languageCommandInterface.get_partner())) + { + LOG_WARNING("[TC]: No response to our request for the language command data, which is unusual."); + + if (nullptr != primaryVirtualTerminal) + { + LOG_WARNING("[TC]: Falling back to VT for language data."); + primaryVirtualTerminal->languageCommandInterface.send_request_language_command(); + stateMachineTimestamp_ms = SystemTiming::get_timestamp_ms(); + } + else + { + LOG_WARNING("[TC]: Since no VT was specified, falling back to a global request for language data."); + languageCommandInterface.set_partner(nullptr); + languageCommandInterface.send_request_language_command(); + stateMachineTimestamp_ms = SystemTiming::get_timestamp_ms(); + } + } } break; diff --git a/test/language_command_interface_tests.cpp b/test/language_command_interface_tests.cpp index d12588132..6429628da 100644 --- a/test/language_command_interface_tests.cpp +++ b/test/language_command_interface_tests.cpp @@ -211,6 +211,9 @@ TEST(LANGUAGE_COMMAND_INTERFACE_TESTS, SettersAndTransmitting) interfaceUnderTest.initialize(); + // Sending a request without setting the various string parameters should not emit a message + EXPECT_FALSE(interfaceUnderTest.send_language_command()); + interfaceUnderTest.set_language_code("en"); interfaceUnderTest.set_commanded_decimal_symbol(LanguageCommandInterface::DecimalSymbols::Comma); interfaceUnderTest.set_commanded_time_format(LanguageCommandInterface::TimeFormats::TwentyFourHour); diff --git a/test/tc_client_tests.cpp b/test/tc_client_tests.cpp index c05b9c170..c9e73c78a 100644 --- a/test/tc_client_tests.cpp +++ b/test/tc_client_tests.cpp @@ -4,6 +4,7 @@ #include "isobus/hardware_integration/virtual_can_plugin.hpp" #include "isobus/isobus/can_network_manager.hpp" #include "isobus/isobus/isobus_task_controller_client.hpp" +#include "isobus/isobus/isobus_virtual_terminal_client.hpp" #include "isobus/utility/system_timing.hpp" #include "helpers/control_function_helpers.hpp" @@ -13,8 +14,8 @@ using namespace isobus; class DerivedTestTCClient : public TaskControllerClient { public: - DerivedTestTCClient(std::shared_ptr partner, std::shared_ptr clientSource) : - TaskControllerClient(partner, clientSource, nullptr){}; + DerivedTestTCClient(std::shared_ptr partner, std::shared_ptr clientSource, std::shared_ptr primaryVT = nullptr) : + TaskControllerClient(partner, clientSource, primaryVT){}; bool test_wrapper_send_working_set_master() const { @@ -1309,11 +1310,11 @@ TEST(TASK_CONTROLLER_CLIENT_TESTS, TimeoutTests) interfaceUnderTest.update(); EXPECT_EQ(interfaceUnderTest.test_wrapper_get_state(), TaskControllerClient::StateMachineState::RequestLanguage); - // Test lack of timeout waiting for language (hold state) + // Test that we can't get stuck in the request language state interfaceUnderTest.test_wrapper_set_state(TaskControllerClient::StateMachineState::WaitForLanguageResponse); EXPECT_EQ(interfaceUnderTest.test_wrapper_get_state(), TaskControllerClient::StateMachineState::WaitForLanguageResponse); interfaceUnderTest.update(); - EXPECT_EQ(interfaceUnderTest.test_wrapper_get_state(), TaskControllerClient::StateMachineState::WaitForLanguageResponse); + EXPECT_EQ(interfaceUnderTest.test_wrapper_get_state(), TaskControllerClient::StateMachineState::ProcessDDOP); // Test timeout waiting for object pool transfer response interfaceUnderTest.test_wrapper_set_state(TaskControllerClient::StateMachineState::WaitForObjectPoolTransferResponse, 0); @@ -1677,3 +1678,79 @@ TEST(TASK_CONTROLLER_CLIENT_TESTS, CallbackTests) ASSERT_TRUE(TestPartnerTC->destroy(3)); ASSERT_TRUE(internalECU->destroy(3)); } + +TEST(TASK_CONTROLLER_CLIENT_TESTS, LanguageCommandFallback) +{ + VirtualCANPlugin serverTC; + serverTC.open(); + + CANHardwareInterface::set_number_of_can_channels(1); + CANHardwareInterface::assign_can_channel_frame_handler(0, std::make_shared()); + CANHardwareInterface::start(); + + auto internalECU = test_helpers::claim_internal_control_function(0xFC, 0); + auto TestPartnerTC = test_helpers::force_claim_partnered_control_function(0xFB, 0); + auto TestPartnerVT = test_helpers::force_claim_partnered_control_function(0xFA, 0); + + auto vtClient = std::make_shared(TestPartnerVT, internalECU); + + DerivedTestTCClient interfaceUnderTest(TestPartnerTC, internalECU, vtClient); + interfaceUnderTest.initialize(false); + vtClient->initialize(false); + + std::this_thread::sleep_for(std::chrono::milliseconds(50)); + + // Get the virtual CAN plugin back to a known state + CANMessageFrame testFrame = {}; + while (!serverTC.get_queue_empty()) + { + serverTC.read_frame(testFrame); + } + ASSERT_TRUE(serverTC.get_queue_empty()); + + auto blankDDOP = std::make_shared(); + interfaceUnderTest.configure(blankDDOP, 1, 32, 32, true, false, true, false, true); + + // Force a status message out of the TC which states it's version 4 + testFrame.identifier = 0x18CBFFFB; + testFrame.data[0] = 0x10; // Mux + testFrame.data[1] = 0x04; // Version number (Version 4) + testFrame.data[2] = 0xFF; // Max boot time (Not available) + testFrame.data[3] = 0x1F; // Supports all options + testFrame.data[4] = 0x00; // Reserved options = 0 + testFrame.data[5] = 0x01; // Number of booms for section control (1) + testFrame.data[6] = 0x20; // Number of sections for section control (32) + testFrame.data[7] = 0x10; // Number channels for position based control (16) + CANNetworkManager::CANNetwork.process_receive_can_message_frame(testFrame); + CANNetworkManager::CANNetwork.update(); + + interfaceUnderTest.test_wrapper_set_state(TaskControllerClient::StateMachineState::RequestLanguage); + ASSERT_EQ(interfaceUnderTest.test_wrapper_get_state(), TaskControllerClient::StateMachineState::RequestLanguage); + interfaceUnderTest.update(); + + serverTC.read_frame(testFrame); + + EXPECT_EQ(testFrame.identifier, 0x18EAFBFC); // Make sure we got the request for language, target the TC + + // Now just sit here and wait for the timeout to occur, 2s + std::this_thread::sleep_for(std::chrono::milliseconds(2001)); + interfaceUnderTest.update(); + interfaceUnderTest.update(); + + // Now we should see another request, this time to the VT + serverTC.read_frame(testFrame); + EXPECT_EQ(testFrame.identifier, 0x18EAFAFC); // Make sure we got the request for language, target the VT + + // Now get really crazy and don't respond to that + std::this_thread::sleep_for(std::chrono::milliseconds(6001)); + interfaceUnderTest.update(); + + // Test that we didn't get stuck in the request language state + EXPECT_EQ(interfaceUnderTest.test_wrapper_get_state(), TaskControllerClient::StateMachineState::ProcessDDOP); + TestPartnerTC->destroy(); + TestPartnerVT->destroy(); + internalECU->destroy(); + + CANHardwareInterface::stop(); + CANNetworkManager::CANNetwork.update(); +}