Re: [Pyode-user] Lots of missing functions added to PyODE

[<al...@ci...>]

> al...@ci... wrote:
>> A summary of changes:
>>
>> * World: damping functions, QuickStep over-relaxation tuning
>> * Body: damping, auto-disable, setMovedCallback
>> * Joints:
>>   - enable/disable joints
>>   - JointType... constants
>>   - 3 new joint classes: Piston, PR and PU.
>
> Hmm.. why did you move the enable/disable functions for bodies to the
> bottom of the file, instead of adding the autodisable stuff where the
> enable stuff was?

Well... at first I wanted to keep (more or less) the same order as ODE wiki.

> I think _bodyMovedCallbacks should be a weakdict, shouldn't it? Also,
> would it make more sense to add a _body_c2py_lut lookup table like for
> geoms in ode.py? In that case you'd only need to store the callbacks in
> _bodyMovedCallbacks. Of course, if it's a weakdict, you'd need to handle
> the case where the body goes out of scope.. presumably the moved
> callback won't be called if the body ceases to exist, but I'm not sure ;)

You are right. I will learn how to use weakdicts :)

> Maybe Body.getData() and Body.setData() should raise NotImplementedError?

Yes, this is also a good idea.

> You add a lot of blank lines between sections of code! I find this
> distracting. Also, you have lots and lots of trailing whitespace, which
> I really hate.

Well.. my coding style includes lots of whitespace :)
Okay, now I found Document->Strip Trailing Spaces in Geany. I did not notice them.

> Also, it seems a little silly for code this simple, but you could send
> smaller patches, especially if you're rearranging method declarations.
>
>> Many declarations were generated automatically with a small script: auto.py
>> (it
>> is attached).
>
> I added this, it's pretty neat. Thanks.
>
>> The following functions are still not implemented (I need some help here):
>>
>> dJointID dBodyGetJoint (dBodyID, int index);  (should return a Joint instance)
>
> I would add a _joint_c2py_lut, like with geoms. Ignore the comment, that
> was before I fully understood why tutorial3 used to break :)
>
>> dGeomID dBodyGetFirstGeom (dBodyID);
>> dGeomID dBodyGetNextGeom (dGeomID);           (these two should be wrapped
>> into
>> an iterable or a geom list)
>
> Hmm, if nobody gets to this before the weekend I'll see if I can add
> something.
>
>> Regards,
>> Alex
>
> How would you like to be credited it the changelog? Just your email address?
>
> Ethan
>

You may credit me like this:

Alex Dumitrache <al...@ci...>


In ode.pyx there is a comment:

## This causes some kind of weird bug! Need to fix this.

import weakref

_geom_c2py_lut = weakref.WeakValueDictionary()


In my programs, sometimes the lut is not able to find the geom ID,
and Space.collide_callback() fails with a KeyError. When this happens,
some boxes pass through the floor and fall down to -infinity.
Using a normal dictionary seems to solve the problem.

Is this the weird bug?


I also tried to implement Set* and Get* methods as properties, they are more
elegant.

    body.mass = 1           instead of body.setMass(1)
    print body.position     instead of print body.getPosition()

However, in Body class (and maybe others), they conflict with __setattr__.

At first, I have tried to override __setattr__, but I wasn't able to call the
default implementation (that's because Body is a "cdef class").

Then, I tried to remove __getattr__ and __setattr__ and instead use
something like this:

a) Add custom data as properties to a dedicated field ("data"):


    # Set custom data

    body.data.mycounter = 5

    body.data.mystring = "test"



    # Access custom data

    print body.data.mystring, body.data.mycounter



b) Supply a custom data object:



   # Set custom data

   body.data = myCustomData


   # Access custom data

   print body.data



Therefore, we have two possibilities to solve the conflict:

A) Overriding __setattr__:

   Pros: - compatible with existing code

   Cons: - may be slow (overriding __getattr__ and __setattr__ slows down the
access to class items)
         - you are not warned at all if you misspell the name of a property
          (e.g. you use body.pos = (1,2,3) but the correct name is
'body.position'; however, python will not complain at all)

B) Custom data field:
   Pros: - more lightweight
         - works fine, with no conflicts
         - if you misspell the name of a property, you will receive an error

   Cons: - breaks existing code which uses custom data

Which one should we choose? I prefer B) since it already works on my computer.

I have also implemented a mechanism for accessing joint parameters as properties.
It works like this:

j = ode.HingeJoint(world)
j.param.lo_stop = 0
j.param.cfm = 1e-5
j.param.vel = 0.1

For complex joints:
joint.param[1].fmax = 1
joint.param[2].fmax = 2
joint.param[3].fmax = 3

And for Plane2D:
joint.paramX.cfm = 1e-5
joint.paramY.cfm = 1e-10
joint.paramAngle.cfm = 1e-10

Or:
joint.param['X'].cfm = 1e-5
joint.param['Y'].cfm = 1e-10
joint.param['Angle'].cfm = 1e-5

And of course, the joint parameters are visible at TAB completion.

The old method, with setParam and getParam, still works as before.

The implementation uses descriptors, and all the functions are in the Joint base
class.
I will do some profiling to see if this does not slowdown the creation of
contact joints
in collision handler (because right now, they are a bottleneck in my robot
simulation).

I'm not sending the patch right now, since the code needs a bit of cleanup.

Alex