Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

RSDK-8836: Gantry Can't Home with Limit Switches #4383

Merged
merged 1 commit into from
Sep 24, 2024

Conversation

martha-johnston
Copy link
Contributor

@martha-johnston martha-johnston commented Sep 23, 2024

when checking if the gantry hit a limit switch during homing, we were also checking if it hit the other limit switch that we didn't expect, and returned an error so the user knows their limit switches aren't configured correctly. this caused an out of range error when there was only 1 limit switch since there isn't a second limit switch to check but we were assuming there was.

I thought there'd be a software test to check this but all the functions, specifically GPIOPinByName are injected so it failed when we told it to and passed when we told it to. looking for suggestions of a good way to add a test for this even now that it's fixed

@martha-johnston martha-johnston requested review from a team and randhid and removed request for a team September 23, 2024 19:28
@viambot viambot added the safe to test This pull request is marked safe to test from a trusted zone label Sep 23, 2024
Copy link
Member

@randhid randhid left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we pass in the limit switch pin name to carry out this logic rather than the pin number, the limit pins are an array of strings after all. So the wrong hits would be defined by the name of the pin.

I'm okay doing this for now, we may need to do a bit of a code refactor to make all of this more testable.

@martha-johnston
Copy link
Contributor Author

Should we pass in the limit switch pin name to carry out this logic rather than the pin number, the limit pins are an array of strings after all. So the wrong hits would be defined by the name of the pin.

I like that because it simplifies this check, but I think it slightly complicates the other instances of checking the pins, so I think you're right it'd be better to change in a future refactor

@martha-johnston martha-johnston merged commit 2374ffb into viamrobotics:main Sep 24, 2024
19 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
safe to test This pull request is marked safe to test from a trusted zone
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants