update all VT client messages to priority 5 done

[dafiliks]

updates all VT client messages to priority 5 as stated in #378

[dafiliks]

I don't know why it's failing...

[ad3154]

I don't know why it's failing...

It's failing because the unit tests for the VT client are comparing the whole CAN ID of the expected message to the actual message that gets sent.

Here you can see that it's checking to see that the ID of this message begins with 0x1C, which is priority 7, but now we're sending it with a leading 0x14, which is priority 5. So, in order to get it to pass, you'd need to update the file test\vt_client_tests.cpp to change the expected ID of the failing messages.

[ad3154]

You can check the output here to see the exact lines you'll have to modify: 919, 936, 952, 968, 979, and 992.

[dafiliks]

Thanks

[dafiliks]

Sorry, I'll do this when I come back from school.

[dafiliks]

Please check if this is what you wanted.

[GwnDaan]

Good job! It'a almost there I think, only thing still left would be the line:

https://github.com/Open-Agriculture/AgIsoStack-plus-plus/blob/main/isobus%2Finclude%2Fisobus%2Fisobus%2Fisobus_virtual_terminal_client.hpp#L1657

Here again priority is not used anymore, so the std::pair can be removed, and you'd end up with the nested std::vector<std::vector<std::uint8_t>>

Also I saw you managed to squash your commits, nice!

[dafiliks]

Hey, I don't know why you would want to remove this, because we want the priority to still exist but it just to be set to 5? Anyways, I did this and the it->first and it->second statements stopped working. I tried to fix this but its most likely wrong. Check it out. Thanks!

[GwnDaan]

Hey, I don't know why you would want to remove this, because we want the priority to still exist but it just to be set to 5? Anyways, I did this and the it->first and it->second statements stopped working. I tried to fix this but its most likely wrong. Check it out. Thanks!

Yeah, so basically the commandQueue was the link between the queue_command function and send_command function. When that link first was created, I thought it could have different priorities, but with the current standard there we're no different priorities for VT commands.

However, that doesn't mean that messages (something different than the VT commands) can't have other priorities, it's just that the commandQueue is only for VT commands and not for messages in general. And since you already changed the send_command to have a fixed priority of 5, I figured we can just remove the priority for the VT commands further up the road, since after that fixed priority, they were basically unused.

I hope this explanation makes sense to you hahah. I'm not sure about your knowledge of the CAN/J1939/ISO11783 protocols in general, but I understand it could be a bit hard to gasp otherwise.

[GwnDaan]

Thanks again for taking the time to contribute to the project. Don't take the feedback personally, I like to keep the project up to a high standard haha. Your efforts are greatly appreciated!