Skip to content

Commit

Permalink
added run_to_abs_pos
Browse files Browse the repository at this point in the history
  • Loading branch information
Anton Vanhoucke authored and Anton Vanhoucke committed Oct 7, 2015
1 parent 7da8f02 commit 345ea13
Showing 1 changed file with 43 additions and 2 deletions.
45 changes: 43 additions & 2 deletions ev3dev/ev3dev.py
Original file line number Diff line number Diff line change
Expand Up @@ -503,6 +503,7 @@ def run_timed(self, time_sp, *args, **kwargs):
:param: time in milliseconds
:param: power 0-100 (duty cycle)
:param speed: speed in
:returns: self (instantly)
"""

Expand All @@ -516,9 +517,49 @@ def run_timed(self, time_sp, *args, **kwargs):
self.__set_command("run-timed")
return self

def run_forever(self,duty_cycle_sp=75):
self.__set_duty_cycle_sp(duty_cycle_sp)
def run_forever(self, *args, **kwargs):
"""
Runs the motor at a certain power until you stop it.
motor.run_forever(50)
Optionally you can use speed regulation and set speed parameter instead.
motor.run_timed(speed=60)
:param: power 0-100 (duty cycle)
:param speed: Speed (in RPM? or deg/sec? or tacho/sec?)
:returns: self (instantly)
"""

if 'speed' in kwargs:
self.__set_speed_regulation_enabled('on')
self.__set_speed_sp(kwargs['speed'])
elif args[0]:
self.__set_speed_regulation_enabled('off')
self.__set_duty_cycle_sp(args[0])
self.__set_command("run-forever")
return self

def run_to_abs_pos(self, position_sp, **kwargs):
"""
Runs the motor to a certain position and then stops.
You can optionally set PID parameters
e.g.
motor.run_to_abs_pos(50, kp=1, ki=0.5, kd=0.2)
:param: position (in tacho's? degrees?)
:returns: self (instantly)
"""

if 'kp' in kwargs:
self.__set_position_p(kwargs['kp'])
if 'ki' in kwargs:
self.__set_position_i(kwargs['ki'])
if 'kd' in kwargs:
self.__set_position_d(kwargs['kd'])

self.__set_position_sp(position_sp)
self.__set_command("run-to-abs-pos")


def stop(self):
self.__set_command("stop")
Expand Down

22 comments on commit 345ea13

@rhempel
Copy link

@rhempel rhempel commented on 345ea13 Oct 8, 2015

Choose a reason for hiding this comment

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

Same here - it's a helpful function, but I'd prefer it in a separate module.

@antonvh
Copy link
Owner

@antonvh antonvh commented on 345ea13 Oct 8, 2015

Choose a reason for hiding this comment

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

I was just making functionality similar to the boostc python lbrary. That one allowed for motor.run_to_abs_pos(50). Why the different architecture?

@rhempel
Copy link

@rhempel rhempel commented on 345ea13 Oct 8, 2015

Choose a reason for hiding this comment

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

The language bindings are generated automatically from the autogen/spec.json file in the ev3dev-lang repo. The spec covers the attributes in the file system that each device can have.

Helper functions beyond these bindings belong in a separate module - I believe that @ddemidov wrote most of the boostc library by hand, so there may not have been the same design goals/constraints.

Besides, if the helper functions are in a separate module, they will be truly independent from the binding implementation :-)

@ddemidov
Copy link

Choose a reason for hiding this comment

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

@rhempel, both C++ and python-boostc fully utilized autogen system. By the way, the spec does require the helper motor functions to be present, so may be they belong to the main module (or at least should be imported by default).

@rhempel
Copy link

@rhempel rhempel commented on 345ea13 Oct 8, 2015

Choose a reason for hiding this comment

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

Oh, now I see the section on helper functions - but we don't have a formal spec for what they are, do we? That means that each binding can implement whatever helpers they want.
I can certainly see the benefit of helpers, but I think they should be specified more formally. Can I ask you to open an issue on adding the helper functions to spec.json?

@ddemidov
Copy link

Choose a reason for hiding this comment

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

The functions were discussed here: ev3dev/ev3dev-lang#89, we can still use the issue. The reason for rather informal specification is that we thought its difficult to specify them so that they would feel natural to each of the bindings languages.

@rhempel
Copy link

@rhempel rhempel commented on 345ea13 Oct 8, 2015

Choose a reason for hiding this comment

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

Thanks @ddemidov, I'll bet we can come up with a spec for this - let's turn some ideas around in our heads...

@antonvh
Copy link
Owner

@antonvh antonvh commented on 345ea13 Oct 8, 2015 via email

Choose a reason for hiding this comment

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

@ddemidov
Copy link

Choose a reason for hiding this comment

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

@antonvh
Copy link
Owner

@antonvh antonvh commented on 345ea13 Oct 8, 2015 via email

Choose a reason for hiding this comment

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

@rhempel
Copy link

@rhempel rhempel commented on 345ea13 Oct 8, 2015

Choose a reason for hiding this comment

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

I'm not sure that you can't spec them - all the functions return a certain type, just like the attributes, and they take parameters that we can specify, and we can add a default value that can be supported by certain languages. The implementation of the helper is usually a sequence of operations on the existing attributes. It should be specifiable - in theory.
In practice, we may use a language specific module to implement the helpers - I'm not saying we can't put helper functions into the ev3dev.py file, just that we should think about alternative implementations as well.

@ddemidov
Copy link

Choose a reason for hiding this comment

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

The arguments are exactly the problem. In python a function can take **kwargs argument, and then you can write

motor.run_timed(speed_regulation_enabled='on', speed_sp=500)

In C++ this is impossible. The closest thing I could come up with is

motor.set_regulation_enabled('on').set_speed_sp(500).run_timed()

which is OK for C++, but is different.

@antonvh
Copy link
Owner

@antonvh antonvh commented on 345ea13 Oct 8, 2015 via email

Choose a reason for hiding this comment

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

@rhempel
Copy link

@rhempel rhempel commented on 345ea13 Oct 8, 2015

Choose a reason for hiding this comment

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

You are right, most people will stick with a preference. So for Python, can we think of a way that a user can do:

import ev3dev

Where ev3dev.py has the helper functions, but it imports ev3dev_binding.py (or similar) where the raw bindings live? Something along those lines?

@ddemidov, looking at the C++ and the python implementation, of run_timed() it may in fact be possible to make that a template...the text tokens are all quite similar.

@ddemidov
Copy link

Choose a reason for hiding this comment

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

We could have a couple of submodules. For example, ev3dev.core would contain 'raw' bindings, ev3dev.extra would contain syntactic sugar, and importing ev3dev would import both.

Re C++ implementation with templates: I don't see how can this be solved with templates, can you elaborate? The function in python may have any number of parameters, and the names of the parameters are nowhere fixed. User can set any device attribute with this.

@rhempel
Copy link

@rhempel rhempel commented on 345ea13 Oct 8, 2015

Choose a reason for hiding this comment

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

@ddemidov, does the command chaining in C++ implementation need to be in a specific order?

@rhempel
Copy link

@rhempel rhempel commented on 345ea13 Oct 8, 2015

Choose a reason for hiding this comment

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

I just pushed a proof of concept here: [https://github.com/ev3dev/ev3dev-lang/tree/pure-python]
Just a basic helper function and a template that ignores parameters and defaults for now. I'll build it out a bit more later - needs a loop to evaluate parameters and build the function header, then an additional section for the attribute settings. Maybe it will not work at all or in general, but at least we tried ...

@rhempel
Copy link

@rhempel rhempel commented on 345ea13 Oct 9, 2015

Choose a reason for hiding this comment

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

I have played around with autogening helper functions and I'm not really happy with the results. My preference now is to have the helper functions written by hand in a separate module and take advantage of the Pythonic way of doing things, like this [http://stackoverflow.com/a/3394902] and this [http://stackoverflow.com/a/36908].
Thanks to @antonvh and @ddemidov for making me learn more about Python and hopefully making me a better architect/programmer.
Besides, the DcMotor devices would need the some of the same helper functions like run_timed() and friends. I think there's a way to create an instance of a MotorHelper class that takes one of the base EV3 motor classes in the __init__ and then figures out which methods apply - but it's too late tonight to figure it out.

@antonvh
Copy link
Owner

@antonvh antonvh commented on 345ea13 Oct 9, 2015 via email

Choose a reason for hiding this comment

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

@ddemidov
Copy link

Choose a reason for hiding this comment

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

@rhempel, re C++ command chaining:
It can support any order. It works like this: each set_* method returns a reference to *this, so it can be immediately followed by any other method of the same class.

@ddemidov
Copy link

Choose a reason for hiding this comment

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

@rhempel, re MotorHelper idea:
The spec.json provides a list of commands for each of the motors. This list is used by both C++ and python-boostc to generate the helper functions. Same approach could be used in pure-python.

@antonvh
Copy link
Owner

@antonvh antonvh commented on 345ea13 Oct 9, 2015

Choose a reason for hiding this comment

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

@rhempel MotorHelper: the usual method to do this is build a superclass for the overlapping functions and subclasses for specific methods. This clashes with the structure of the generated code, though. So I'm afraid some repetition will occur.

Please sign in to comment.