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

Workaround bug in the GG by forcing the pins to be updated in order #31

Merged
merged 4 commits into from
Jun 12, 2022

Conversation

shardros
Copy link
Member

@shardros shardros commented Nov 7, 2021

One OO solution for #29

Does make some interfaces tigheter coupled but there is no nice way to do this which I can think of.s

My GG does not seem to be responding to commands to GPIO on master so not sure what is going on there. Not tested this branch as a result.

@shardros shardros requested a review from WillMunns November 7, 2021 12:03
@WillMunns
Copy link
Contributor

I've made a couple of changes to work with this new scheme. reset.py appears to instantiate bits of the greengiant on demand from Shepherd. It assumed that the GPIO were stateless, so I have changed it to make the full list, and update through that. Let me know if I have understood this incorrectly - it appears to work on a brain.

Originally the state of the pin internally was configured as None - presumably to stop people poking GPIO that they had not touched. This breaks now that every touch updates everything. The change to default this to INPUT rather than None matches reality, but I'm not certain its best to do it here, or to check for None in the setter and set it to INPUT there and would keep some of the "you haven't explicitly configured this yet" behavior.

To ensure that state of the GG matches the state assumed locally at init
@shardros
Copy link
Member Author

Added update in init to ensure that the state of the GG does actually match the new assumed state of INPUT. Tested PR with:

import robot
import time

R = robot.Robot()

R.gpio[1].mode = robot.OUTPUT
R.gpio[2].mode = robot.OUTPUT
R.gpio[3].mode = robot.OUTPUT
R.gpio[4].mode = robot.OUTPUT

R.gpio[1].digital = False
R.gpio[2].digital = False
R.gpio[3].digital = False
R.gpio[4].digital = False

print("Configured outputs")

time.sleep(10)

R.gpio[1].digital = True
R.gpio[2].digital = True
R.gpio[3].digital = True
R.gpio[4].digital = True

print("Configured outputs")

time.sleep(10)

R.gpio[2].mode = robot.INPUT

Happy to merge.

@shardros
Copy link
Member Author

@WillMunns Not sure if you have seen but I am happy for this to be merged to master now.

@shardros
Copy link
Member Author

@WillMunns Should this be merged? Was it already patched in. Does master reflect the current state of the brains?

@shardros
Copy link
Member Author

Given that we are now in the summer merge window, I think that this was merged in the end I am just going to merge this so that any other branches have it

@shardros shardros merged commit 2c23e77 into master Jun 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants