Skip to content

Remove call to _update_coils in __init__ #29

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

Merged
merged 2 commits into from
May 31, 2019

Conversation

MrYsLab
Copy link

@MrYsLab MrYsLab commented May 27, 2019

The reason for this pull request is documented here: adafruit/Adafruit_CircuitPython_Crickit#17.

@ladyada
Copy link
Member

ladyada commented May 27, 2019

awesome, thank you! could you verify it still works with stepper motors as expected?

@MrYsLab
Copy link
Author

MrYsLab commented May 27, 2019

Yes, and it works if I install dc motors or a stepper. Tested on a Raspberry PI 3B+

@ladyada
Copy link
Member

ladyada commented May 27, 2019

rad, thanks - the tests no longer pass because they assume that the coils start energized. if you are up for updating those as well, then when it passes we can merge it

@MrYsLab
Copy link
Author

MrYsLab commented May 27, 2019

I am not able to run the tests in my environment but looking at the Travis output, it appears what needs to be done is to remove the following lines from each test:

for j in range(4):
    assert coil[j].duty_cycle == (0xffff if j == 0 else 0)

If that is correct, I can edit the test-file but want to make sure that is all that needs to be done and that I don't create havoc by doing so. Just let me know if you wish me to do that.

@ladyada
Copy link
Member

ladyada commented May 27, 2019

i think maybe you want to perform a step - im not sure to be honest. you can leave it and when someone has a chance they can try fixing the test code up

@MrYsLab
Copy link
Author

MrYsLab commented May 27, 2019

Sounds good to me. Tnx.

@makermelissa
Copy link
Collaborator

makermelissa commented May 27, 2019

One option you could do is place an additional parameter into the init with a default of True, such as init_coils=True which would call _update_coils if True. You could override the default behavior by passing init_coils=False into your constructor and the tests should pass without modification.

@ladyada
Copy link
Member

ladyada commented May 27, 2019

im ok with them not activating at first until the first step, id rather do that and stick with the decision than make it optional (and confusing?)

@makermelissa
Copy link
Collaborator

Ok, makes sense. That'll work then.

@ladyada
Copy link
Member

ladyada commented May 27, 2019

wanna take a look at fixing the asserts at your convience?

@makermelissa
Copy link
Collaborator

Yep :)

@makermelissa makermelissa self-requested a review May 27, 2019 21:36
@makermelissa
Copy link
Collaborator

Ok, it turns out uninitialized they should return all zeros, so I adjusted the asserts to compare against that.

@makermelissa makermelissa merged commit 36ac241 into adafruit:master May 31, 2019
@MrYsLab
Copy link
Author

MrYsLab commented May 31, 2019

Thank you.

adafruit-adabot added a commit to adafruit/Adafruit_CircuitPython_Bundle that referenced this pull request May 31, 2019
@tannewt
Copy link
Member

tannewt commented May 31, 2019

I'm rolling this back. The library isn't designed to use when something else is connected.

More details here: adafruit/Adafruit_CircuitPython_Crickit#17 (comment)

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.

4 participants