RoboJackets / robocup-firmware

Georgia Tech RoboJackets Firmware for the RoboCup Small Size League
Apache License 2.0
27 stars 20 forks source link

Fix Corruption at End of RTP Bot to Soccer Packet #50

Closed jpfeltracco closed 6 years ago

guyfleeman commented 6 years ago

@jpfeltracco got any ideas what's going on?

ashaw596 commented 6 years ago

FYI if it helps. I think I found last year it doesn't happen in the version before the last major radio refractor they did before comp. There were alot of big changes though so it might not help.

jgkamat commented 6 years ago

Yup, last I checked I think Evan said he fixed this, so I think this might just be stale...

On June 3, 2018 8:26:43 PM EDT, ashaw596 notifications@github.com wrote:

FYI if it helps. I think I found last year it doesn't happen in the version before the last major radio refractor they did before comp. There were alot of big changes though so it might not help.

-- Sent from my Android device with K-9 Mail. Please excuse my brevity.

guyfleeman commented 6 years ago

@ashaw596 thanks for the info @petersonev any updates?

JNeiger commented 6 years ago

I think this is still happening. When @jpfeltracco was implementing the encoder data forwarding to soccer, we had to add a buffer at the end so the last encoder wouldn't get corrupted.

Edit: I just noticed the date on the issue. This was opened because we had to do that.

guyfleeman commented 6 years ago

@JNeiger so wait, you understand the issue?

ashaw596 commented 6 years ago

Just looking through the code base a bit so not sure. Could it be this line? Not sure whats its suppose to do but it was added a year ago. The comment says its suppose to delete the last two elements. (Idk why though) It actually deletes the 3rd and second to last elements and leaves the last element.

// remove the last 2 elements buf.erase(buf.end() - 3, buf.end() - 1);

https://github.com/RoboJackets/robocup-firmware/blob/d51a6d3b0b03ffa08991147a7b44068ad39fd75f/lib/drivers/decawave/Decawave.cpp#L139

JNeiger commented 6 years ago

@guyfleeman I'm not sure what is causing it. I just know that when we were doing the encoder forwarding, this issue popped up.

At the very least, if you remove that buffer value in the packet, it should be really easy to test if the problem still persists.

guyfleeman commented 6 years ago

@jneiger what branch is current for motion control data where we can try to reproduce?

guyfleeman commented 6 years ago

@ashaw596 good catch. I'll try to reproduce soon and see if modifying that line fixes it

JNeiger commented 6 years ago

@guyfleeman I don't think we made a specific branch to make testing the issue easy. The encoder stuff should already be on master. "Theoretically" just removing the buffer item from the packet should be enough to easily notice the problem when you print the encoder values to the console.

These are the relevant PRs for this https://github.com/RoboJackets/robocup-firmware/pull/55 https://github.com/RoboJackets/robocup-software/pull/1129 https://github.com/RoboJackets/robocup-fshare/pull/1

jpfeltracco commented 6 years ago

Yeah, just remove the dummy packet field called "buffer" or something and look at encoder 4 in the tree under the RX packet item. It won't make sense.