patches for the motion_control stack

hi,

I've some patches for the motion_control stack:
1) stack dependency renaming
2) there was a fishy duration calculation
3) clean up of ugly event names, to something iTaSC (and I) can live with (underscore between component name and actual event content, as used everywhere else): this will
have some impact for people using it!!!!

ciao
nick

patches for the motion_control stack

Hi Nick,

Thanks for the patches.

On Tue, Jan 22, 2013 at 11:31:31AM +0100, Dominick Vanthienen wrote:
>
> I've some patches for the motion_control stack:
> 1) stack dependency renaming

This has been fixed in

commit 0fe2fc13ab3c3febe2adaba2c1bafde753379cb1
Author: Markus Klotzbuecher <markus [dot] klotzbuecher [..] ...>
Date: Thu Jun 28 12:52:59 2012 +0200

Rename orocos_toolchain_ros to orocos_toolchain.

Thanks to Mirko Comparetti for reporting.

stack.xml | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

> 2) there was a fishy duration calculation

This patch does not seem to address a fishing duration calculation,
but changes the starting position of a new moveTo from the position of
the last trajectory to current position!? This makes sense, unless
anyone objects I will apply (but please provide a proper description
next time)

> 3) clean up of ugly event names, to something iTaSC (and I) can live
> with (underscore between component name and actual event content, as
> used everywhere else): this will have some impact for people using
> it!!!!

Can you check out Bert's patch, who cleans this up (and also adds a
move_started event)? Can you live with that too?

Thanks!
Markus

patches for the motion_control stack

On 01/24/2013 03:02 PM, Markus Klotzbuecher wrote:
> Hi Nick,
>
> Thanks for the patches.
>
> On Tue, Jan 22, 2013 at 11:31:31AM +0100, Dominick Vanthienen wrote:
>>
>> I've some patches for the motion_control stack:
>> 1) stack dependency renaming
>
> This has been fixed in
>
> commit 0fe2fc13ab3c3febe2adaba2c1bafde753379cb1
> Author: Markus Klotzbuecher <markus [dot] klotzbuecher [..] ...>
> Date: Thu Jun 28 12:52:59 2012 +0200
>
> Rename orocos_toolchain_ros to orocos_toolchain.
>
> Thanks to Mirko Comparetti for reporting.
>
> stack.xml | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
m, ok, I did a diff with the current version and this came up...
>
>> 2) there was a fishy duration calculation
>
> This patch does not seem to address a fishing duration calculation,
> but changes the starting position of a new moveTo from the position of
> the last trajectory to current position!? This makes sense, unless
> anyone objects I will apply (but please provide a proper description
> next time)
it _is_ fishy: here's the description:
p_d is the desired position value that you send out
as long as you haven't moved yet: p_d.positions = joint_state.position = what you get from p_m_port=the measured joint state
but if you did move, p_d != joint_state
some KDl api: virtual void SetProfileDuration (double start_pos, double end_pos, double newduration)
position= the position you want to generate a trajectory to

the code:
---------
if (!is_moving){
max_duration = 0;
// get current position/
p_m_port.read( joint_state );=>this line is added by the patch, to now where you are at the moment
for (unsigned int i=0; i<num_axes; i++){
// Set motion profiles
motion_profile[i].SetProfileDuration( p_d.positions[i], position[i], time );
=>you want to calculate a profile from p_d to position, but p_d is the last value you sent out maybe ages ago, this isn't necessary equal to joint_state
// Find lengthiest trajectory
max_duration = max( max_duration, motion_profile[i].Duration() );
}
// Rescale trajectories to maximal duration
log(Info)<<"Moving to [";
for(unsigned int i = 0; i < num_axes; i++){
motion_profile[i].SetProfileDuration( joint_state.position[i], position[i], max_duration );
=>the proposed solution (should also be applied in the one above, my patch didn't do that) you want to move from where you are to where you want to go (makes sense, no?)
log(Info)<<position[i]<<" ";
}
}

conclusion:
the unpatched code does only work as long as p_d=p_m=joint_state, it doesn't even read the current state when moveTo is called

>
>> 3) clean up of ugly event names, to something iTaSC (and I) can live
>> with (underscore between component name and actual event content, as
>> used everywhere else): this will have some impact for people using
>> it!!!!
>
> Can you check out Bert's patch, who cleans this up (and also adds a
> move_started event)? Can you live with that too?
ok, same changes
>
> Thanks!
> Markus
>

patches for the motion_control stack

On 01/24/2013 03:31 PM, Dominick Vanthienen wrote:
>
>
> On 01/24/2013 03:02 PM, Markus Klotzbuecher wrote:
>> Hi Nick,
>>
>> Thanks for the patches.
>>
>> On Tue, Jan 22, 2013 at 11:31:31AM +0100, Dominick Vanthienen wrote:
>>>
>>> I've some patches for the motion_control stack:
>>> 1) stack dependency renaming
>>
>> This has been fixed in
>>
>> commit 0fe2fc13ab3c3febe2adaba2c1bafde753379cb1
>> Author: Markus Klotzbuecher <markus [dot] klotzbuecher [..] ...>
>> Date: Thu Jun 28 12:52:59 2012 +0200
>>
>> Rename orocos_toolchain_ros to orocos_toolchain.
>>
>> Thanks to Mirko Comparetti for reporting.
>>
>> stack.xml | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
> m, ok, I did a diff with the current version and this came up...
>>
>>> 2) there was a fishy duration calculation
>>
>> This patch does not seem to address a fishing duration calculation,
>> but changes the starting position of a new moveTo from the position of
>> the last trajectory to current position!? This makes sense, unless
>> anyone objects I will apply (but please provide a proper description
>> next time)
> it _is_ fishy: here's the description:
> p_d is the desired position value that you send out
> as long as you haven't moved yet: p_d.positions = joint_state.position = what you get from p_m_port=the measured joint state
> but if you did move, p_d != joint_state
> some KDl api: virtual void SetProfileDuration (double start_pos, double end_pos, double newduration)
> position= the position you want to generate a trajectory to
>
> the code:
> ---------
> if (!is_moving){
> max_duration = 0;
> // get current position/
> p_m_port.read( joint_state );=>this line is added by the patch, to now where you are at the moment
> for (unsigned int i=0; i<num_axes; i++){
> // Set motion profiles
> motion_profile[i].SetProfileDuration( p_d.positions[i], position[i], time );
> =>you want to calculate a profile from p_d to position, but p_d is the last value you sent out maybe ages ago, this isn't necessary equal to joint_state
> // Find lengthiest trajectory
> max_duration = max( max_duration, motion_profile[i].Duration() );
> }
> // Rescale trajectories to maximal duration
> log(Info)<<"Moving to [";
> for(unsigned int i = 0; i < num_axes; i++){
> motion_profile[i].SetProfileDuration( joint_state.position[i], position[i], max_duration );
> =>the proposed solution (should also be applied in the one above, my patch didn't do that) you want to move from where you are to where you want to go (makes sense, no?)
extra patch for this one ;)
> log(Info)<<position[i]<<" ";
> }
> }
>
> conclusion:
> the unpatched code does only work as long as p_d=p_m=joint_state, it doesn't even read the current state when moveTo is called
>
>>
>>> 3) clean up of ugly event names, to something iTaSC (and I) can live
>>> with (underscore between component name and actual event content, as
>>> used everywhere else): this will have some impact for people using
>>> it!!!!
>>
>> Can you check out Bert's patch, who cleans this up (and also adds a
>> move_started event)? Can you live with that too?
> ok, same changes
>>
>> Thanks!
>> Markus
>>