Open-Agriculture / AgIsoStack-plus-plus

AgIsoStack++ is the completely free open-source C++ ISOBUS library for everyone
https://agisostack.com/
MIT License
187 stars 41 forks source link

[TC]: Add restart and reupload capabilities to TC client #363

Closed ad3154 closed 10 months ago

ad3154 commented 10 months ago

What's new?

Closes #354

Closes #355

ad3154 commented 10 months ago

Formatting check depends on #364, and I'll try and throw in a couple trivial unit tests but it's still essentially ready for review.

trm01 commented 10 months ago

I just tried this branch with 2 different TCs and saw the same behavior. The re-upload works well, much quicker than the tear-down and re-initialization. Great so far, however after the re-upload I start to see that the TC is requesting values for what I think is the incorrect elements. My 2nd DDOP re-uses element numbers (maybe that is not allowed? ) and in my RequestValueCommandCallback is getting DDIs requests for a element number that never presented it as an option in the 2nd DDOP.

Am I doing something wrong, or are these element numbers reset within the stack or TC server?

ad3154 commented 10 months ago

Oh hey, good catch, it's probably because we call that function based on the content of previous requests from the TC, and I'm probably not clearing them on a reset - I will push an update to this branch that will probably fix that

ad3154 commented 10 months ago

@trm01 Updated in 0db0a542a12c67756e63c1cbd7496282445b0e9d if you have time to re-test.

trm01 commented 10 months ago

Awesome, thanks! I won't be able to retest tonight, but will retest tomorrow and get back to you.

sonarcloud[bot] commented 10 months ago

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

77.2% 77.2% Coverage
0.0% 0.0% Duplication

idea Catch issues before they fail your Quality Gate with our IDE extension sonarlint SonarLint

trm01 commented 10 months ago

FYI: I gave this a try and I am still seeing the callbacks. I'm currently digging into the code right now to ensure it isn't something that I am messing up with my DDOP changes.

Edit: I just noticed the following message:

Timeout waiting for deactivate object pool response. This is unusual, but we're just going to reconnect anyways.

trm01 commented 10 months ago

Okay, we're good. I'm confirming it was something I am doing in my code... I was calling the re-upload twice in a row very quickly with 2 different DDOPs. The second one was not being uploaded as the TC client state machine had already started the 1st deactivation.