RSDK-8878: gantry shouldn't look for the second limit switch until it has cleared the first limit switch #4400
Conversation
viambot
added
the
safe to test
…he next limit switch
martha-johnston
force-pushed
the
rsdk-8878
branch
from
70bb4c3
to
c44fbe2
Compare
viambot
added
safe to test
martha-johnston
requested review from
a team and
oliviamiller
and removed request for
a team
viambot
added
safe to test
viambot
added
safe to test
|
fixing the test was more involved than anticipated, could you take another look @oliviamiller? maybe just react to this if you still approve? |
| pinValues = []int{1, 1, 0} | ||
| count = 0 | ||
| defaultPinValues = []int{1, 1, 0} | ||
| limitPinValues = []int{1, 0, 1, 0, 1, 1, 0} |
There was a problem hiding this comment.
I don't really understand what the difference between these two are / why these test changes are needed for this code change, could you give me some more context/add comments to this file?
There was a problem hiding this comment.
yeah I can add comments, but basically we added the moveAway function to homing, so in order for the tests to pass the pin needs to go positive (hit the limit switch) and then negative (moved away from the limit switch). I didn't think about it before but we might just be able to use that limit pin values list for everything. let me try it quickly
There was a problem hiding this comment.
thank you! maybe rename back to pinValues since theres now only one. Also comment is still unclear since they are not all alternating?
There was a problem hiding this comment.
they can be fully alternating, it doesn't super matter it's just that the first couple need to be alternating for the homing tests. basically the issue before was they were never getting set low. at some point it needs to be set low. changed it to fully alternating tho because that is clearer
There was a problem hiding this comment.
thinking to leave it as default because if they need to be changed for some reason in adding future tests, more versions could be added
There was a problem hiding this comment.
maybe just add "during homing" to the comment for future clarity? otherwise everything lgtm!
viambot
added
safe to test
viambot
added
safe to test
viambot
added
safe to test
use move away to make sure limit switch it cleared before homing to the next limit switch
more info from ticket description:
Currently during the gantry homing process, the gantry will move to limit switch 0, and then limit switch 1. During this process, it is constantly checking if it has hit the limit switch it is looking for, but it is also constantly checking if it has hit the wrong limit switch, meaning if it was expecting to hit limit switch 0 next and it hits limit switch 1, or if it expects to hit limit switch 1 next and hits limit switch 0. If this happens, the homing sequence errors and stops.
This causes a problem with slower gantries because there’s a case in which the first limit switch could be hit, and then the gantry starts moving in the opposite direction trying to find the second limit switch, but it has not moved out of the range of the first limit switch yet. In this scenario, the homing sequence is interrupted because the gantry is looking for limit switch 1, but it is still hitting limit switch 0, so it returns an error.
This can be fixed by moving in the opposite direction to set the limit switch back to false before continuing to home in the opposite direction.