Update supported version in working set maintenance message

[GwnDaan]

Currently we ask the application to provide the "supported" VT version for a pool. This is not what the standard want us to provide. It instead requires us to set the VT version in the maintenance message to the version of the standard that the application is designed to.

I think the correct thing to do is provide the standard version the application is designed to comply with. Since the stack is basically the one doing communication defined in the standard, that means that we can put it to a constant version. Namely we aim to be complaint with VT version 6, i.e. ISO11783-6:2018.

Reference to applicable code:

AgIsoStack-plus-plus/isobus/src/isobus_virtual_terminal_client.cpp

Lines 1767 to 1819 in 4951955

	bool VirtualTerminalClient::send_working_set_maintenance(bool initializing, VTVersion workingSetVersion) const
	{
	std::uint8_t versionByte;
	std::uint8_t bitmask = (initializing ? 0x01 : 0x00);
	switch (workingSetVersion)
	{
	case VTVersion::Version3:
	{
	versionByte = 0x03;
	}
	break;
	case VTVersion::Version4:
	{
	versionByte = 0x04;
	}
	break;
	case VTVersion::Version5:
	{
	versionByte = 0x05;
	}
	break;
	case VTVersion::Version6:
	{
	versionByte = 0x06;
	}
	break;
	default:
	{
	versionByte = 0xFF;
	}
	break;
	}
	const std::array<std::uint8_t, CAN_DATA_LENGTH> buffer = { static_cast<std::uint8_t>(Function::WorkingSetMaintenanceMessage),
	bitmask,
	versionByte,
	0xFF,
	0xFF,
	0xFF,
	0xFF,
	0xFF };
	return CANNetworkManager::CANNetwork.send_can_message(static_cast<std::uint32_t>(CANLibParameterGroupNumber::ECUtoVirtualTerminal),
	buffer.data(),
	CAN_DATA_LENGTH,
	myControlFunction,
	partnerControlFunction,
	CANIdentifier::PriorityLowest7);
	}

[ad3154]

We will have to monitor what happens when we do this. Although I agree with it, I have seen some VT servers do some pretty funky stuff, and we should recall that clients are required to change their behavior based on the server version, so it may be less obvious to consumers now that version is something they need to be thinking about. We should be sure that our logging statements are high quality for situations where the server version < client.

[GwnDaan]

Yeah I see your concerns, and they are all valid ones for sure. Do you reckon we should wait with this change? I do think it's a bit more complicated than the phrase I initially put:

Since the stack is basically the one doing communication defined in the standard, that means that we can put it to a constant version. Namely we aim to be complaint with VT version 6, i.e. ISO11783-6:2018.

But I think if we implement the standard more thoroughly in the coming months, we should be fully backwards compatible? Also I'm sure if I understand how this change can mess up a connection with a VT server (if it follows the standard haha)?

[ad3154]

if it follows the standard

I'm not going to name any names... but I have seen some... things in my days... It is not reasonable to try to cover non-conformant terminals probably.

we should be fully backwards compatible

The main thing we cannot control is what the user puts in the object pool, and what functions they call, both of which may result in non-conformant behavior, so we may need to add some additional validation as we implement the standard more thoroughly in the coming months to minimize this possibility?

[ad3154]

It is not reasonable to try to cover non-conformant terminals probably

(Except AgIsoVirtualTerminal hehe)

[ad3154]

I think it's OK to make the change now, but we'll then want to prioritize follow-up refactoring in the client for anything we're not conformant on, such as TAN processing for version 6 servers

[GwnDaan]

Yeah sounds good, let's make some issues to try to upgrade our compatibility

[GwnDaan]

[The main thing we cannot control is] what functions they[the application] calls

We could limit the outgoing messages based on the VT version connected right? E.g. we can make functions like send_lock_unlock_mask (Introduced in VT version 4) always return false if used when connected to a VT version 3 or prior. Or would that be undesirable?

[GwnDaan]

The main thing we cannot control is what the user puts in the object pool

Yeah I think it's also not our job to validate it within the stack. Probably nice to make an open-source tool to test conformance someday to provide a way of validating, but other than that we have to trust their good will. We can however require them to supply requirements for the pool, see #376, so they at least think about it...

[ad3154]

always return false if used when connected to a VT version 3 or prior

This makes sense I think as long as we make a corresponding log statement, otherwise people may be confused