Skip to content

Commit

Permalink
[TC]: Fixed TC client process variable value to be signed
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@ad3154
ad3154 committed Apr 20, 2024
1 parent 2c58692 commit 617cc21
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 50 deletions.
2 changes: 1 addition & 1 deletion examples/seeder_example/section_control_implement_sim.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ bool SectionControlImplementSimulator::create_ddop(std::shared_ptr<isobus::Devic

bool SectionControlImplementSimulator::request_value_command_callback(std::uint16_t,
std::uint16_t DDI,
std::uint32_t &value,
std::int32_t &value,
void *parentPointer)
{
if (nullptr != parentPointer)
Expand Down
2 changes: 1 addition & 1 deletion examples/seeder_example/section_control_implement_sim.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ class SectionControlImplementSimulator
/// @param[in] parentPointer A pointer to the class instance this callback is for
static bool request_value_command_callback(std::uint16_t elementNumber,
std::uint16_t DDI,
std::uint32_t &value,
std::int32_t &value,
void *parentPointer);

/// @brief A callback that will be used by the TC client to set values
Expand Down
10 changes: 5 additions & 5 deletions isobus/include/isobus/isobus/isobus_task_controller_client.hpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ namespace isobus
/// @brief A callback for handling a value request command from the TC
using RequestValueCommandCallback = bool (*)(std::uint16_t elementNumber,
std::uint16_t DDI,
std::uint32_t &processVariableValue,
std::int32_t &processVariableValue,
void *parentPointer);

/// @brief A callback for handling a set value command from the TC
Expand All @@ -112,7 +112,7 @@ namespace isobus

/// @brief This adds a callback that will be called when the TC requests the value of one of your variables.
/// @details The task controller will often send a request for the value of a process data variable.
/// When the stack recieves those messages, it will call this callback to request the value from your
/// When the stack receives those messages, it will call this callback to request the value from your
/// application. You must provide the value at that time for the associated process data variable identified
/// by its element number and DDI.
/// @param[in] callback The callback to add
Expand Down Expand Up @@ -520,7 +520,7 @@ namespace isobus
/// @param[in] ddi The DDI for the command
/// @param[in] value The value to send
/// @returns `true` if the message was sent, otherwise `false`
bool send_value_command(std::uint16_t elementNumber, std::uint16_t ddi, std::uint32_t value) const;
bool send_value_command(std::uint16_t elementNumber, std::uint16_t ddi, std::int32_t value) const;

/// @brief Sends the version request message to the TC
/// @returns `true` if the message was sent, otherwise `false`
Expand Down Expand Up @@ -575,8 +575,8 @@ namespace isobus
/// @param obj the object to compare against
/// @returns true if the ddi and element numbers of the provided objects match, otherwise false
bool operator==(const ProcessDataCallbackInfo &obj) const;
std::uint32_t processDataValue; ///< The value of the value set command
std::uint32_t lastValue; ///< Used for measurement commands to store timestamp or previous values
std::int32_t processDataValue; ///< The value of the value set command
std::int32_t lastValue; ///< Used for measurement commands to store timestamp or previous values
std::uint16_t elementNumber; ///< The element number for the command
std::uint16_t ddi; ///< The DDI for the command
bool ackRequested; ///< Stores if the TC used the mux that also requires a PDACK
Expand Down
74 changes: 37 additions & 37 deletions isobus/src/isobus_task_controller_client.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1040,7 +1040,7 @@ namespace isobus

for (auto &currentCallback : requestValueCallbacks)
{
std::uint32_t newValue = 0;
std::int32_t newValue = 0;
if (currentCallback.callback(currentRequest.elementNumber, currentRequest.ddi, newValue, currentCallback.parent))
{
transmitSuccessful = send_value_command(currentRequest.elementNumber, currentRequest.ddi, newValue);
Expand Down Expand Up @@ -1076,13 +1076,13 @@ namespace isobus

for (auto &measurementTimeCommand : measurementTimeIntervalCommands)
{
if (SystemTiming::time_expired_ms(measurementTimeCommand.lastValue, measurementTimeCommand.processDataValue))
if (SystemTiming::time_expired_ms(static_cast<std::uint32_t>(measurementTimeCommand.lastValue), static_cast<std::uint32_t>(measurementTimeCommand.processDataValue)))
{
// Time to update this time interval variable
transmitSuccessful = false;
for (auto &currentCallback : 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);
Expand All @@ -1092,14 +1092,14 @@ namespace isobus

if (transmitSuccessful)
{
measurementTimeCommand.lastValue = SystemTiming::get_timestamp_ms();
measurementTimeCommand.lastValue = static_cast<std::int32_t>(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 &currentCallback : requestValueCallbacks)
{
if (currentCallback.callback(measurementMaxCommand.elementNumber, measurementMaxCommand.ddi, newValue, currentCallback.parent))
Expand Down Expand Up @@ -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 &currentCallback : requestValueCallbacks)
{
if (currentCallback.callback(measurementMinCommand.elementNumber, measurementMinCommand.ddi, newValue, currentCallback.parent))
Expand Down Expand Up @@ -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 &currentCallback : requestValueCallbacks)
{
if (currentCallback.callback(measurementChangeCommand.elementNumber, measurementChangeCommand.ddi, newValue, currentCallback.parent))
Expand Down Expand Up @@ -1566,10 +1566,10 @@ namespace isobus
requestData.elementNumber = (static_cast<std::uint16_t>(messageData[0] >> 4) | (static_cast<std::uint16_t>(messageData[1]) << 4));
requestData.ddi = static_cast<std::uint16_t>(messageData[2]) |
(static_cast<std::uint16_t>(messageData[3]) << 8);
requestData.processDataValue = (static_cast<std::uint32_t>(messageData[4]) |
(static_cast<std::uint16_t>(messageData[5]) << 8) |
(static_cast<std::uint16_t>(messageData[6]) << 16) |
(static_cast<std::uint16_t>(messageData[7]) << 24));
requestData.processDataValue = (static_cast<std::int32_t>(messageData[4]) |
(static_cast<std::int32_t>(messageData[5]) << 8) |
(static_cast<std::int32_t>(messageData[6]) << 16) |
(static_cast<std::int32_t>(messageData[7]) << 24));
parentTC->queuedValueRequests.push_back(requestData);
}
break;
Expand All @@ -1583,10 +1583,10 @@ namespace isobus
requestData.elementNumber = (static_cast<std::uint16_t>(messageData[0] >> 4) | (static_cast<std::uint16_t>(messageData[1]) << 4));
requestData.ddi = static_cast<std::uint16_t>(messageData[2]) |
(static_cast<std::uint16_t>(messageData[3]) << 8);
requestData.processDataValue = (static_cast<std::uint32_t>(messageData[4]) |
(static_cast<std::uint16_t>(messageData[5]) << 8) |
(static_cast<std::uint16_t>(messageData[6]) << 16) |
(static_cast<std::uint16_t>(messageData[7]) << 24));
requestData.processDataValue = (static_cast<std::int32_t>(messageData[4]) |
(static_cast<std::int32_t>(messageData[5]) << 8) |
(static_cast<std::int32_t>(messageData[6]) << 16) |
(static_cast<std::int32_t>(messageData[7]) << 24));
parentTC->queuedValueCommands.push_back(requestData);
}
break;
Expand All @@ -1600,10 +1600,10 @@ namespace isobus
requestData.elementNumber = (static_cast<std::uint16_t>(messageData[0] >> 4) | (static_cast<std::uint16_t>(messageData[1]) << 4));
requestData.ddi = static_cast<std::uint16_t>(messageData[2]) |
(static_cast<std::uint16_t>(messageData[3]) << 8);
requestData.processDataValue = (static_cast<std::uint32_t>(messageData[4]) |
(static_cast<std::uint16_t>(messageData[5]) << 8) |
(static_cast<std::uint16_t>(messageData[6]) << 16) |
(static_cast<std::uint16_t>(messageData[7]) << 24));
requestData.processDataValue = (static_cast<std::int32_t>(messageData[4]) |
(static_cast<std::int32_t>(messageData[5]) << 8) |
(static_cast<std::int32_t>(messageData[6]) << 16) |
(static_cast<std::int32_t>(messageData[7]) << 24));
parentTC->queuedValueCommands.push_back(requestData);
}
break;
Expand All @@ -1616,11 +1616,11 @@ namespace isobus
commandData.elementNumber = (static_cast<std::uint16_t>(messageData[0] >> 4) | (static_cast<std::uint16_t>(messageData[1]) << 4));
commandData.ddi = static_cast<std::uint16_t>(messageData[2]) |
(static_cast<std::uint16_t>(messageData[3]) << 8);
commandData.processDataValue = (static_cast<std::uint32_t>(messageData[4]) |
(static_cast<std::uint16_t>(messageData[5]) << 8) |
(static_cast<std::uint16_t>(messageData[6]) << 16) |
(static_cast<std::uint16_t>(messageData[7]) << 24));
commandData.lastValue = SystemTiming::get_timestamp_ms();
commandData.processDataValue = (static_cast<std::int32_t>(messageData[4]) |
(static_cast<std::int32_t>(messageData[5]) << 8) |
(static_cast<std::int32_t>(messageData[6]) << 16) |
(static_cast<std::int32_t>(messageData[7]) << 24));
commandData.lastValue = static_cast<std::int32_t>(SystemTiming::get_timestamp_ms());

auto previousCommand = std::find(parentTC->measurementTimeIntervalCommands.begin(), parentTC->measurementTimeIntervalCommands.end(), commandData);
if (parentTC->measurementTimeIntervalCommands.end() == previousCommand)
Expand Down Expand Up @@ -1657,10 +1657,10 @@ namespace isobus
commandData.elementNumber = (static_cast<std::uint16_t>(messageData[0] >> 4) | (static_cast<std::uint16_t>(messageData[1]) << 4));
commandData.ddi = static_cast<std::uint16_t>(messageData[2]) |
(static_cast<std::uint16_t>(messageData[3]) << 8);
commandData.processDataValue = (static_cast<std::uint32_t>(messageData[4]) |
(static_cast<std::uint16_t>(messageData[5]) << 8) |
(static_cast<std::uint16_t>(messageData[6]) << 16) |
(static_cast<std::uint16_t>(messageData[7]) << 24));
commandData.processDataValue = (static_cast<std::int32_t>(messageData[4]) |
(static_cast<std::int32_t>(messageData[5]) << 8) |
(static_cast<std::int32_t>(messageData[6]) << 16) |
(static_cast<std::int32_t>(messageData[7]) << 24));

auto previousCommand = std::find(parentTC->measurementMaximumThresholdCommands.begin(), parentTC->measurementMaximumThresholdCommands.end(), commandData);
if (parentTC->measurementMaximumThresholdCommands.end() == previousCommand)
Expand Down Expand Up @@ -1690,10 +1690,10 @@ namespace isobus
commandData.elementNumber = (static_cast<std::uint16_t>(messageData[0] >> 4) | (static_cast<std::uint16_t>(messageData[1]) << 4));
commandData.ddi = static_cast<std::uint16_t>(messageData[2]) |
(static_cast<std::uint16_t>(messageData[3]) << 8);
commandData.processDataValue = (static_cast<std::uint32_t>(messageData[4]) |
(static_cast<std::uint16_t>(messageData[5]) << 8) |
(static_cast<std::uint16_t>(messageData[6]) << 16) |
(static_cast<std::uint16_t>(messageData[7]) << 24));
commandData.processDataValue = (static_cast<std::int32_t>(messageData[4]) |
(static_cast<std::int32_t>(messageData[5]) << 8) |
(static_cast<std::int32_t>(messageData[6]) << 16) |
(static_cast<std::int32_t>(messageData[7]) << 24));

auto previousCommand = std::find(parentTC->measurementMinimumThresholdCommands.begin(), parentTC->measurementMinimumThresholdCommands.end(), commandData);
if (parentTC->measurementMinimumThresholdCommands.end() == previousCommand)
Expand Down Expand Up @@ -1723,10 +1723,10 @@ namespace isobus
commandData.elementNumber = (static_cast<std::uint16_t>(messageData[0] >> 4) | (static_cast<std::uint16_t>(messageData[1]) << 4));
commandData.ddi = static_cast<std::uint16_t>(messageData[2]) |
(static_cast<std::uint16_t>(messageData[3]) << 8);
commandData.processDataValue = (static_cast<std::uint32_t>(messageData[4]) |
(static_cast<std::uint16_t>(messageData[5]) << 8) |
(static_cast<std::uint16_t>(messageData[6]) << 16) |
(static_cast<std::uint16_t>(messageData[7]) << 24));
commandData.processDataValue = (static_cast<std::int32_t>(messageData[4]) |
(static_cast<std::int32_t>(messageData[5]) << 8) |
(static_cast<std::int32_t>(messageData[6]) << 16) |
(static_cast<std::int32_t>(messageData[7]) << 24));

auto previousCommand = std::find(parentTC->measurementOnChangeThresholdCommands.begin(), parentTC->measurementOnChangeThresholdCommands.end(), commandData);
if (parentTC->measurementOnChangeThresholdCommands.end() == previousCommand)
Expand Down Expand Up @@ -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<std::uint8_t, CAN_DATA_LENGTH> buffer = { static_cast<std::uint8_t>(static_cast<std::uint8_t>(ProcessDataCommands::Value) |
(static_cast<std::uint8_t>(elementNumber & 0x0F) << 4)),
Expand Down
31 changes: 25 additions & 6 deletions test/tc_client_tests.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand All @@ -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;
Expand Down

0 comments on commit 617cc21

Please sign in to comment.