Skip to content

Commit

Permalink
[TC]: Removed TC client's dependency on the VT client
Browse files Browse the repository at this point in the history 
Removed references to the VT client in the TC client, since all it was
used for was the language command, which we can accomplish by only
passing the control function of the VT in instead.
  • Loading branch information
@ad3154
ad3154 committed Mar 22, 2024
1 parent 0778ae7 commit ab2e8a9
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 25 deletions.
9 changes: 5 additions & 4 deletions isobus/include/isobus/isobus/isobus_task_controller_client.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@

namespace isobus
{
class VirtualTerminalClient; // Forward declaring VT client

/// @brief A class to manage a client connection to a ISOBUS field computer's task controller or data logger
class TaskControllerClient
{
Expand Down Expand Up @@ -101,7 +99,7 @@ namespace isobus
/// @param[in] partner The TC server control function
/// @param[in] clientSource The internal control function to communicate from
/// @param[in] primaryVT Pointer to our primary VT. This is optional (can be nullptr), but should be provided if possible to provide the best compatibility to TC < version 4.
TaskControllerClient(std::shared_ptr<PartneredControlFunction> partner, std::shared_ptr<InternalControlFunction> clientSource, std::shared_ptr<VirtualTerminalClient> primaryVT);
TaskControllerClient(std::shared_ptr<PartneredControlFunction> partner, std::shared_ptr<InternalControlFunction> clientSource, std::shared_ptr<PartneredControlFunction> primaryVT);

/// @brief Destructor for the client
~TaskControllerClient();
Expand Down Expand Up @@ -560,6 +558,9 @@ namespace isobus
/// @param[in] timestamp The new value for the state machine timestamp (in milliseconds)
void set_state(StateMachineState newState, std::uint32_t timestamp);

/// @brief Sets the behavior of the language command interface based on the TC's reported version information
void select_language_command_partner();

/// @brief The worker thread will execute this function when it runs, if applicable
void worker_thread_function();

Expand Down Expand Up @@ -614,7 +615,7 @@ namespace isobus

std::shared_ptr<PartneredControlFunction> partnerControlFunction; ///< The partner control function this client will send to
std::shared_ptr<InternalControlFunction> myControlFunction; ///< The internal control function the client uses to send from
std::shared_ptr<VirtualTerminalClient> primaryVirtualTerminal; ///< A pointer to the primary VT. Used for TCs < version 4
std::shared_ptr<PartneredControlFunction> primaryVirtualTerminal; ///< A pointer to the primary VT's control function. Used for TCs < version 4 and language command compatibility
std::shared_ptr<DeviceDescriptorObjectPool> clientDDOP; ///< Stores the DDOP for upload to the TC (if needed)
std::uint8_t const *userSuppliedBinaryDDOP = nullptr; ///< Stores a client-provided DDOP if one was provided
std::shared_ptr<std::vector<std::uint8_t>> userSuppliedVectorDDOP; ///< Stores a client-provided DDOP if one was provided
Expand Down
37 changes: 23 additions & 14 deletions isobus/src/isobus_task_controller_client.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
#include "isobus/isobus/can_general_parameter_group_numbers.hpp"
#include "isobus/isobus/can_network_manager.hpp"
#include "isobus/isobus/can_stack_logger.hpp"
#include "isobus/isobus/isobus_virtual_terminal_client.hpp"
#include "isobus/utility/system_timing.hpp"
#include "isobus/utility/to_string.hpp"

Expand All @@ -25,7 +24,7 @@

namespace isobus
{
TaskControllerClient::TaskControllerClient(std::shared_ptr<PartneredControlFunction> partner, std::shared_ptr<InternalControlFunction> clientSource, std::shared_ptr<VirtualTerminalClient> primaryVT) :
TaskControllerClient::TaskControllerClient(std::shared_ptr<PartneredControlFunction> partner, std::shared_ptr<InternalControlFunction> clientSource, std::shared_ptr<PartneredControlFunction> primaryVT) :
languageCommandInterface(clientSource, partner),
partnerControlFunction(partner),
myControlFunction(clientSource),
Expand Down Expand Up @@ -483,6 +482,7 @@ namespace isobus
if (SystemTiming::time_expired_ms(stateMachineTimestamp_ms, SIX_SECOND_TIMEOUT_MS))
{
LOG_WARNING("[TC]: Timeout waiting for version request from TC. This is not required, so proceeding anways.");
select_language_command_partner();
set_state(StateMachineState::RequestLanguage);
}
}
Expand All @@ -492,6 +492,7 @@ namespace isobus
{
if (send_request_version_response())
{
select_language_command_partner();
set_state(StateMachineState::RequestLanguage);
}
else if (SystemTiming::time_expired_ms(stateMachineTimestamp_ms, TWO_SECOND_TIMEOUT_MS))
Expand All @@ -504,17 +505,7 @@ namespace isobus

case StateMachineState::RequestLanguage:
{
if ((serverVersion < static_cast<std::uint8_t>(Version::SecondPublishedEdition)) &&
(nullptr == primaryVirtualTerminal))
{
languageCommandInterface.set_partner(nullptr); // TC might not reply and no VT specified, so just see if anyone knows.
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<std::uint8_t>(Version::SecondPublishedEdition)) &&
(nullptr != primaryVirtualTerminal) &&
(primaryVirtualTerminal->languageCommandInterface.send_request_language_command())) ||
(languageCommandInterface.send_request_language_command()))
if (languageCommandInterface.send_request_language_command())
{
set_state(StateMachineState::WaitForLanguageResponse);
languageCommandWaitingTimestamp_ms = SystemTiming::get_timestamp_ms();
Expand Down Expand Up @@ -547,7 +538,8 @@ namespace isobus
if (nullptr != primaryVirtualTerminal)
{
LOG_WARNING("[TC]: Falling back to VT for language data.");
primaryVirtualTerminal->languageCommandInterface.send_request_language_command();
languageCommandInterface.set_partner(primaryVirtualTerminal);
languageCommandInterface.send_request_language_command();
stateMachineTimestamp_ms = SystemTiming::get_timestamp_ms();
}
else
Expand Down Expand Up @@ -2083,6 +2075,23 @@ namespace isobus
currentState = newState;
}

void TaskControllerClient::select_language_command_partner()
{
if (serverVersion < static_cast<std::uint8_t>(Version::SecondPublishedEdition))
{
if (nullptr == primaryVirtualTerminal)
{
languageCommandInterface.set_partner(nullptr); // TC might not reply and no VT specified, so just see if anyone knows.
LOG_WARNING("[TC]: The TC is < version 4 but no VT was provided. Language data will be requested globally, which might not be ideal.");
}
else
{
languageCommandInterface.set_partner(primaryVirtualTerminal);
LOG_DEBUG("[TC]: Using VT as the partner for language data, because the TC's version is less than 4.");
}
}
}

void TaskControllerClient::worker_thread_function()
{
#if !defined CAN_STACK_DISABLE_THREADS && !defined ARDUINO
Expand Down
11 changes: 4 additions & 7 deletions test/tc_client_tests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ using namespace isobus;
class DerivedTestTCClient : public TaskControllerClient
{
public:
DerivedTestTCClient(std::shared_ptr<PartneredControlFunction> partner, std::shared_ptr<InternalControlFunction> clientSource, std::shared_ptr<VirtualTerminalClient> primaryVT = nullptr) :
DerivedTestTCClient(std::shared_ptr<PartneredControlFunction> partner, std::shared_ptr<InternalControlFunction> clientSource, std::shared_ptr<PartneredControlFunction> primaryVT = nullptr) :
TaskControllerClient(partner, clientSource, primaryVT){};

bool test_wrapper_send_working_set_master() const
Expand Down Expand Up @@ -1113,7 +1113,7 @@ TEST(TASK_CONTROLLER_CLIENT_TESTS, StateMachineTests)

CANNetworkManager::CANNetwork.update(); //! @todo: quick hack for clearing the transmit queue, can be removed once network manager' singleton is removed
//! @todo try to reduce the reference count, such that that we don't use a control function after it is destroyed
ASSERT_TRUE(tcPartner->destroy(4));
ASSERT_TRUE(tcPartner->destroy(5));
ASSERT_TRUE(internalECU->destroy(5));
}

Expand Down Expand Up @@ -1339,7 +1339,7 @@ TEST(TASK_CONTROLLER_CLIENT_TESTS, TimeoutTests)
EXPECT_EQ(interfaceUnderTest.test_wrapper_get_state(), TaskControllerClient::StateMachineState::Disconnected);

//! @todo try to reduce the reference count, such that that we don't use a control function after it is destroyed
ASSERT_TRUE(tcPartner->destroy(3));
tcPartner->destroy(5);
ASSERT_TRUE(internalECU->destroy(3));
}

Expand Down Expand Up @@ -1692,11 +1692,8 @@ TEST(TASK_CONTROLLER_CLIENT_TESTS, LanguageCommandFallback)
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<VirtualTerminalClient>(TestPartnerVT, internalECU);

DerivedTestTCClient interfaceUnderTest(TestPartnerTC, internalECU, vtClient);
DerivedTestTCClient interfaceUnderTest(TestPartnerTC, internalECU, TestPartnerVT);
interfaceUnderTest.initialize(false);
vtClient->initialize(false);

std::this_thread::sleep_for(std::chrono::milliseconds(50));

Expand Down

0 comments on commit ab2e8a9

Please sign in to comment.