From 7c6733b5f6d5575e2e7e583e224e54da4baaeaa0 Mon Sep 17 00:00:00 2001 From: Adrian Del Grosso <10929341+ad3154@users.noreply.github.com> Date: Sat, 20 Apr 2024 11:27:42 -0600 Subject: [PATCH] [TC]: Fixed TC client process variable value to be signed Fixed casing issues when determining the TC client process variable values. Changed TC client process variable value to be a signed value, as it should be according to the definition in ISO 11783-10 B.3.4 --- .../section_control_implement_sim.cpp | 4 +- .../section_control_implement_sim.hpp | 4 +- .../section_control_implement_sim.cpp | 4 +- .../section_control_implement_sim.hpp | 4 +- .../isobus/isobus_task_controller_client.hpp | 12 +-- isobus/src/isobus_task_controller_client.cpp | 74 +++++++++---------- test/tc_client_tests.cpp | 33 +++++++-- 7 files changed, 77 insertions(+), 58 deletions(-) diff --git a/examples/seeder_example/section_control_implement_sim.cpp b/examples/seeder_example/section_control_implement_sim.cpp index 0bd965325..efd4ed4c4 100644 --- a/examples/seeder_example/section_control_implement_sim.cpp +++ b/examples/seeder_example/section_control_implement_sim.cpp @@ -218,7 +218,7 @@ bool SectionControlImplementSimulator::create_ddop(std::shared_ptr(measurementTimeCommand.lastValue), static_cast(measurementTimeCommand.processDataValue))) { // Time to update this time interval variable transmitSuccessful = false; for (auto ¤tCallback : requestValueCallbacks) { - std::uint32_t newValue = 0; + std::int32_t newValue = 0; if (currentCallback.callback(measurementTimeCommand.elementNumber, measurementTimeCommand.ddi, newValue, currentCallback.parent)) { transmitSuccessful = send_value_command(measurementTimeCommand.elementNumber, measurementTimeCommand.ddi, newValue); @@ -1092,14 +1092,14 @@ namespace isobus if (transmitSuccessful) { - measurementTimeCommand.lastValue = SystemTiming::get_timestamp_ms(); + measurementTimeCommand.lastValue = static_cast(SystemTiming::get_timestamp_ms()); } } } for (auto &measurementMaxCommand : measurementMaximumThresholdCommands) { // Get the current process data value - std::uint32_t newValue = 0; + std::int32_t newValue = 0; for (auto ¤tCallback : requestValueCallbacks) { if (currentCallback.callback(measurementMaxCommand.elementNumber, measurementMaxCommand.ddi, newValue, currentCallback.parent)) @@ -1127,7 +1127,7 @@ namespace isobus for (auto &measurementMinCommand : measurementMinimumThresholdCommands) { // Get the current process data value - std::uint32_t newValue = 0; + std::int32_t newValue = 0; for (auto ¤tCallback : requestValueCallbacks) { if (currentCallback.callback(measurementMinCommand.elementNumber, measurementMinCommand.ddi, newValue, currentCallback.parent)) @@ -1155,7 +1155,7 @@ namespace isobus for (auto &measurementChangeCommand : measurementOnChangeThresholdCommands) { // Get the current process data value - std::uint32_t newValue = 0; + std::int32_t newValue = 0; for (auto ¤tCallback : requestValueCallbacks) { if (currentCallback.callback(measurementChangeCommand.elementNumber, measurementChangeCommand.ddi, newValue, currentCallback.parent)) @@ -1566,10 +1566,10 @@ namespace isobus requestData.elementNumber = (static_cast(messageData[0] >> 4) | (static_cast(messageData[1]) << 4)); requestData.ddi = static_cast(messageData[2]) | (static_cast(messageData[3]) << 8); - requestData.processDataValue = (static_cast(messageData[4]) | - (static_cast(messageData[5]) << 8) | - (static_cast(messageData[6]) << 16) | - (static_cast(messageData[7]) << 24)); + requestData.processDataValue = (static_cast(messageData[4]) | + (static_cast(messageData[5]) << 8) | + (static_cast(messageData[6]) << 16) | + (static_cast(messageData[7]) << 24)); parentTC->queuedValueRequests.push_back(requestData); } break; @@ -1583,10 +1583,10 @@ namespace isobus requestData.elementNumber = (static_cast(messageData[0] >> 4) | (static_cast(messageData[1]) << 4)); requestData.ddi = static_cast(messageData[2]) | (static_cast(messageData[3]) << 8); - requestData.processDataValue = (static_cast(messageData[4]) | - (static_cast(messageData[5]) << 8) | - (static_cast(messageData[6]) << 16) | - (static_cast(messageData[7]) << 24)); + requestData.processDataValue = (static_cast(messageData[4]) | + (static_cast(messageData[5]) << 8) | + (static_cast(messageData[6]) << 16) | + (static_cast(messageData[7]) << 24)); parentTC->queuedValueCommands.push_back(requestData); } break; @@ -1600,10 +1600,10 @@ namespace isobus requestData.elementNumber = (static_cast(messageData[0] >> 4) | (static_cast(messageData[1]) << 4)); requestData.ddi = static_cast(messageData[2]) | (static_cast(messageData[3]) << 8); - requestData.processDataValue = (static_cast(messageData[4]) | - (static_cast(messageData[5]) << 8) | - (static_cast(messageData[6]) << 16) | - (static_cast(messageData[7]) << 24)); + requestData.processDataValue = (static_cast(messageData[4]) | + (static_cast(messageData[5]) << 8) | + (static_cast(messageData[6]) << 16) | + (static_cast(messageData[7]) << 24)); parentTC->queuedValueCommands.push_back(requestData); } break; @@ -1616,11 +1616,11 @@ namespace isobus commandData.elementNumber = (static_cast(messageData[0] >> 4) | (static_cast(messageData[1]) << 4)); commandData.ddi = static_cast(messageData[2]) | (static_cast(messageData[3]) << 8); - commandData.processDataValue = (static_cast(messageData[4]) | - (static_cast(messageData[5]) << 8) | - (static_cast(messageData[6]) << 16) | - (static_cast(messageData[7]) << 24)); - commandData.lastValue = SystemTiming::get_timestamp_ms(); + commandData.processDataValue = (static_cast(messageData[4]) | + (static_cast(messageData[5]) << 8) | + (static_cast(messageData[6]) << 16) | + (static_cast(messageData[7]) << 24)); + commandData.lastValue = static_cast(SystemTiming::get_timestamp_ms()); auto previousCommand = std::find(parentTC->measurementTimeIntervalCommands.begin(), parentTC->measurementTimeIntervalCommands.end(), commandData); if (parentTC->measurementTimeIntervalCommands.end() == previousCommand) @@ -1657,10 +1657,10 @@ namespace isobus commandData.elementNumber = (static_cast(messageData[0] >> 4) | (static_cast(messageData[1]) << 4)); commandData.ddi = static_cast(messageData[2]) | (static_cast(messageData[3]) << 8); - commandData.processDataValue = (static_cast(messageData[4]) | - (static_cast(messageData[5]) << 8) | - (static_cast(messageData[6]) << 16) | - (static_cast(messageData[7]) << 24)); + commandData.processDataValue = (static_cast(messageData[4]) | + (static_cast(messageData[5]) << 8) | + (static_cast(messageData[6]) << 16) | + (static_cast(messageData[7]) << 24)); auto previousCommand = std::find(parentTC->measurementMaximumThresholdCommands.begin(), parentTC->measurementMaximumThresholdCommands.end(), commandData); if (parentTC->measurementMaximumThresholdCommands.end() == previousCommand) @@ -1690,10 +1690,10 @@ namespace isobus commandData.elementNumber = (static_cast(messageData[0] >> 4) | (static_cast(messageData[1]) << 4)); commandData.ddi = static_cast(messageData[2]) | (static_cast(messageData[3]) << 8); - commandData.processDataValue = (static_cast(messageData[4]) | - (static_cast(messageData[5]) << 8) | - (static_cast(messageData[6]) << 16) | - (static_cast(messageData[7]) << 24)); + commandData.processDataValue = (static_cast(messageData[4]) | + (static_cast(messageData[5]) << 8) | + (static_cast(messageData[6]) << 16) | + (static_cast(messageData[7]) << 24)); auto previousCommand = std::find(parentTC->measurementMinimumThresholdCommands.begin(), parentTC->measurementMinimumThresholdCommands.end(), commandData); if (parentTC->measurementMinimumThresholdCommands.end() == previousCommand) @@ -1723,10 +1723,10 @@ namespace isobus commandData.elementNumber = (static_cast(messageData[0] >> 4) | (static_cast(messageData[1]) << 4)); commandData.ddi = static_cast(messageData[2]) | (static_cast(messageData[3]) << 8); - commandData.processDataValue = (static_cast(messageData[4]) | - (static_cast(messageData[5]) << 8) | - (static_cast(messageData[6]) << 16) | - (static_cast(messageData[7]) << 24)); + commandData.processDataValue = (static_cast(messageData[4]) | + (static_cast(messageData[5]) << 8) | + (static_cast(messageData[6]) << 16) | + (static_cast(messageData[7]) << 24)); auto previousCommand = std::find(parentTC->measurementOnChangeThresholdCommands.begin(), parentTC->measurementOnChangeThresholdCommands.end(), commandData); if (parentTC->measurementOnChangeThresholdCommands.end() == previousCommand) @@ -2002,7 +2002,7 @@ namespace isobus partnerControlFunction); } - bool TaskControllerClient::send_value_command(std::uint16_t elementNumber, std::uint16_t ddi, std::uint32_t value) const + bool TaskControllerClient::send_value_command(std::uint16_t elementNumber, std::uint16_t ddi, std::int32_t value) const { const std::array buffer = { static_cast(static_cast(ProcessDataCommands::Value) | (static_cast(elementNumber & 0x0F) << 4)), diff --git a/test/tc_client_tests.cpp b/test/tc_client_tests.cpp index b699b854f..d45ab26dd 100644 --- a/test/tc_client_tests.cpp +++ b/test/tc_client_tests.cpp @@ -1372,11 +1372,11 @@ static std::uint16_t requestedDDI = 0; static std::uint16_t commandedDDI = 0; static std::uint16_t requestedElement = 0; static std::uint16_t commandedElement = 0; -static std::uint32_t commandedValue = 0; +static std::int32_t commandedValue = 0; bool request_value_command_callback(std::uint16_t element, std::uint16_t ddi, - std::uint32_t &, + std::int32_t &, void *) { requestedElement = element; @@ -1387,7 +1387,7 @@ bool request_value_command_callback(std::uint16_t element, bool value_command_callback(std::uint16_t element, std::uint16_t ddi, - std::uint32_t value, + std::int32_t value, void *) { commandedElement = element; @@ -1511,6 +1511,29 @@ TEST(TASK_CONTROLLER_CLIENT_TESTS, CallbackTests) valueRequested = false; requestedDDI = 0; requestedElement = 0; + + // Test negative number + testFrame.identifier = 0x18CB86F7; + testFrame.data[0] = 0x2A; + testFrame.data[1] = 0x05; + testFrame.data[2] = 0x29; + testFrame.data[3] = 0x48; + testFrame.data[4] = 0x11; + testFrame.data[5] = 0x01; + testFrame.data[6] = 0x00; + testFrame.data[7] = 0xF0; + CANNetworkManager::CANNetwork.process_receive_can_message_frame(testFrame); + CANNetworkManager::CANNetwork.update(); + interfaceUnderTest.update(); + + EXPECT_EQ(true, valueCommanded); + EXPECT_EQ(commandedDDI, 0x4829); + EXPECT_EQ(commandedElement, 0x52); + EXPECT_EQ(commandedValue, -268435183); + + valueCommanded = false; + commandedDDI = 0; + commandedValue = 0; interfaceUnderTest.remove_request_value_callback(request_value_command_callback, nullptr); // Create a request for a value. @@ -1530,10 +1553,6 @@ TEST(TASK_CONTROLLER_CLIENT_TESTS, CallbackTests) EXPECT_EQ(false, valueRequested); EXPECT_EQ(requestedDDI, 0); EXPECT_EQ(requestedElement, 0); - EXPECT_EQ(true, valueCommanded); - EXPECT_EQ(commandedDDI, 0x4829); - EXPECT_EQ(commandedElement, 0x52); - EXPECT_EQ(commandedValue, 0x5060708); valueCommanded = false; commandedDDI = 0;