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

Even more suggestions #3

Open
Redoubts opened this issue Jul 25, 2014 · 1 comment
Open

Even more suggestions #3

Redoubts opened this issue Jul 25, 2014 · 1 comment

Comments

@Redoubts
Copy link

Hello!

I came across your project from the EV3dev page, like most people I assume. I took a look at some of the suggestions others made, and incorporated theirs with a few of my own.

https://github.com/Redoubts/python-ev3

I didn't clone the repo, as I'm only interested in ev3.py for my purposes, but you could probably do a local diff to see what changed.

Highlights are better use of pyudev so that sensor selection is more generic, merging brake/halt mode into a single stop mode, and also a few sensible defaults for motion rotating. I also unguarded some exceptions, because I'd rather have my mistakes thrown in my face right away -- but that's just my opinion.

Thanks for your work! It was very clear code, and worked well as a companion to the ev3dev docs.

@cavenel
Copy link
Owner

cavenel commented Jul 26, 2014

Thank you for your great job, I really like your modifications. (and I learned about for-else blocks!)
The idea of using pyudev is good. I was going to use it as well after seeing the code from https://gist.github.com/dlech/11098915
I am not sure about your way of coding set_speed. It forces you to use set_speed after using set_regulation, which is not obvious to me (and which is not the case in your "rotate_forever" function. Maybe using set_pulses_per_second_sp and set_duty_cycle_sp functions (and then choosing one or the other in the rotate functions according to the "regulate" parameter) would be more understandable?

I will merge all of your changes in a near future. As they contain the tweaks from @ensonic (except for the beeping feature that I will add), it is perfect.

I think I will add a way to select sensor by port, and see how to deal with multiple sensors with same type_id (if you want to use 2 push buttons for example).

Thank you again!

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

No branches or pull requests

2 participants