proposed patch for orogen/typegen

I'm proposing these non-intrusive changes to the cmake code of typegen
such that we can install the transport libs into an
orocos/target/pkgname/types subdirectory instead of in orocos/types.
This allows our deployer to cleanly import these typekit packages.
Before, they were always imported.

It's not clear yet to me what I'm breaking with this patch so please review :-)

Also, I had a question regarding typegen -i pkgname, where 'pkgname'
is also typegen generated:
It seems that it picks up the .tlb files, but still the code for the
transport of these earlier defined types is generated. Can typegen be
changed such that for these -i specified packages, no code is
generated anymore ?

Peter

AttachmentSize
0001-typekit-allow-to-generate-typekit-libs-into-target-p.patch7.06 KB

proposed patch for orogen/typegen

On 01/31/2013 05:44 PM, Peter Soetens wrote:
> Also, I had a question regarding typegen -i pkgname, where 'pkgname'
> is also typegen generated:
> It seems that it picks up the .tlb files, but still the code for the
> transport of these earlier defined types is generated. Can typegen be
> changed such that for these -i specified packages, no code is
> generated anymore ?
It is a typegen-specific issue, orogen does show the "right" behaviour.

I'm having a look and I am puzzled. Could you send the .pc file for the
imported typekit ?

proposed patch for orogen/typegen

On Fri, Feb 1, 2013 at 2:05 PM, Sylvain Joyeux <sylvain [dot] joyeux [..] ...> wrote:
> On 01/31/2013 05:44 PM, Peter Soetens wrote:
>>
>> Also, I had a question regarding typegen -i pkgname, where 'pkgname'
>> is also typegen generated:
>> It seems that it picks up the .tlb files, but still the code for the
>> transport of these earlier defined types is generated. Can typegen be
>> changed such that for these -i specified packages, no code is
>> generated anymore ?
>
> It is a typegen-specific issue, orogen does show the "right" behaviour.
>
> I'm having a look and I am puzzled. Could you send the .pc file for the
> imported typekit ?

This patch on typegen fixes it:

$ di
diff --git a/bin/typegen b/bin/typegen
index 517448f..971a0e7 100755

proposed patch for orogen/typegen

Ah AH! Now I remember why I was so puzzled.

None of the git versions of typegen I looked (rock master, orocos
master, orocos toolchain 2.4, 2.5 and 2.6) have this !pkg.typelist. I
guess someone added it there "because it was nil" ('typelist'
disappeared for quite a long time), hiding the actual bug.

Where did this come from ? What version are you using ?

Now the code is correctly generated, but I don't know if this is not
obfuscating a deeper-down bug...

Nope. This patch is doing the right thing. You just should not test for
!empty?. Commit that to master, I'll take care of fixing -x.

Sylvain

proposed patch for orogen/typegen

On Fri, Feb 1, 2013 at 2:45 PM, Sylvain Joyeux <sylvain [dot] joyeux [..] ...> wrote:
> Ah AH! Now I remember why I was so puzzled.
>
> None of the git versions of typegen I looked (rock master, orocos master,
> orocos toolchain 2.4, 2.5 and 2.6) have this !pkg.typelist. I guess someone
> added it there "because it was nil" ('typelist' disappeared for quite a long
> time), hiding the actual bug.
>
> Where did this come from ? What version are you using ?

Sorry, patch I made yesterday on my local copy:-) It was indeed nil,
so I added a check. Indeed, the check must be there since the user can
provide a library, in which case it may be nill.

>
>
> Now the code is correctly generated, but I don't know if this is not
> obfuscating a deeper-down bug...
>
> Nope. This patch is doing the right thing. You just should not test for
> !empty?. Commit that to master, I'll take care of fixing -x.

Ok, got it.

Peter

proposed patch for orogen/typegen

On Fri, Feb 1, 2013 at 2:49 PM, Peter Soetens <peter [..] ...> wrote:
> On Fri, Feb 1, 2013 at 2:45 PM, Sylvain Joyeux <sylvain [dot] joyeux [..] ...> wrote:
>> Ah AH! Now I remember why I was so puzzled.
>>
>> None of the git versions of typegen I looked (rock master, orocos master,
>> orocos toolchain 2.4, 2.5 and 2.6) have this !pkg.typelist. I guess someone
>> added it there "because it was nil" ('typelist' disappeared for quite a long
>> time), hiding the actual bug.
>>
>> Where did this come from ? What version are you using ?
>
> Sorry, patch I made yesterday on my local copy:-) It was indeed nil,
> so I added a check. Indeed, the check must be there since the user can
> provide a library, in which case it may be nill.
>
>>
>>
>> Now the code is correctly generated, but I don't know if this is not
>> obfuscating a deeper-down bug...
>>
>> Nope. This patch is doing the right thing. You just should not test for
>> !empty?. Commit that to master, I'll take care of fixing -x.
>
> Ok, got it.

https://www.gitorious.org/orocos-toolchain/orogen/merge_requests/1

I created a repo clone of orogen and then a merge request... It would
have been more convenient to clone rock-orogen, then you could see two
more patches regarding TAO/OMNIORB cmake fixes...

I could also merge rock-orogen/master into orogen/master if you
consider it stable...ie the other way around.

I omitted the OROCOS_TARGET stuff.

Peter

proposed patch for orogen/typegen

On Fri, Feb 1, 2013 at 3:16 PM, Peter Soetens <peter [..] ...> wrote:
> On Fri, Feb 1, 2013 at 2:49 PM, Peter Soetens <peter [..] ...> wrote:
>> On Fri, Feb 1, 2013 at 2:45 PM, Sylvain Joyeux <sylvain [dot] joyeux [..] ...> wrote:
>>> Ah AH! Now I remember why I was so puzzled.
>>>
>>> None of the git versions of typegen I looked (rock master, orocos master,
>>> orocos toolchain 2.4, 2.5 and 2.6) have this !pkg.typelist. I guess someone
>>> added it there "because it was nil" ('typelist' disappeared for quite a long
>>> time), hiding the actual bug.
>>>
>>> Where did this come from ? What version are you using ?
>>
>> Sorry, patch I made yesterday on my local copy:-) It was indeed nil,
>> so I added a check. Indeed, the check must be there since the user can
>> provide a library, in which case it may be nill.
>>
>>>
>>>
>>> Now the code is correctly generated, but I don't know if this is not
>>> obfuscating a deeper-down bug...
>>>
>>> Nope. This patch is doing the right thing. You just should not test for
>>> !empty?. Commit that to master, I'll take care of fixing -x.
>>
>> Ok, got it.
>
> https://www.gitorious.org/orocos-toolchain/orogen/merge_requests/1
>
> I created a repo clone of orogen and then a merge request... It would
> have been more convenient to clone rock-orogen, then you could see two
> more patches regarding TAO/OMNIORB cmake fixes...
>
> I could also merge rock-orogen/master into orogen/master if you
> consider it stable...ie the other way around.
>
> I omitted the OROCOS_TARGET stuff.

Ping ? Can we / you merge this ?

Peter

proposed patch for orogen/typegen

On 02/07/2013 11:17 AM, Peter Soetens wrote:
>
> Ping ? Can we / you merge this ?
Did not have time to test this... Things are pretty chaotic for me right
now. Sorry. I'll try to do it over the week end.

Sylvain

proposed patch for orogen/typegen

On Fri, Feb 8, 2013 at 10:40 AM, Sylvain Joyeux <sylvain [dot] joyeux [..] ...> wrote:
> On 02/07/2013 11:17 AM, Peter Soetens wrote:
>>
>>
>> Ping ? Can we / you merge this ?
>
> Did not have time to test this... Things are pretty chaotic for me right
> now. Sorry. I'll try to do it over the week end.

Maybe this weekend :-) ?

Peter

proposed patch for orogen/typegen

On 02/16/2013 01:25 AM, Peter Soetens wrote:
> Maybe this weekend :-) ?
I made a comment on the merge request ...

Let's make it again here :P

Why did you enable the new scheme only when using OrocosRTT CMake
macros, and not as soon as OROCOS_TARGET is set ?

proposed patch for orogen/typegen

On 02/18/2013 10:40 AM, Sylvain Joyeux wrote:
> On 02/16/2013 01:25 AM, Peter Soetens wrote:
>> Maybe this weekend :-) ?
> I made a comment on the merge request ...
>
> Let's make it again here :P
>
> Why did you enable the new scheme only when using OrocosRTT CMake
> macros, and not as soon as OROCOS_TARGET is set ?
Ping ?

proposed patch for orogen/typegen

On Tue, Mar 19, 2013 at 3:21 PM, Sylvain Joyeux <sylvain [dot] joyeux [..] ...> wrote:
> On 02/18/2013 10:40 AM, Sylvain Joyeux wrote:
>>
>> On 02/16/2013 01:25 AM, Peter Soetens wrote:
>>>
>>> Maybe this weekend :-) ?
>>
>> I made a comment on the merge request ...
>>
>> Let's make it again here :P
>>
>> Why did you enable the new scheme only when using OrocosRTT CMake
>> macros, and not as soon as OROCOS_TARGET is set ?
>
> Ping ?

Sorry..

I didn't dare to change it for 'everyone' since I couldn't easily
test nor estimate how much this would affect the existing code base.

If you don't see any harm, then be my guest to unify this for all cases.

Cheers,
Peter

proposed patch for orogen/typegen

On 03/21/2013 11:44 PM, Peter Soetens wrote:
> If you don't see any harm, then be my guest to unify this for all cases.
I'll try that and come back to you.

proposed patch for orogen/typegen

On Fri, Feb 1, 2013 at 2:05 PM, Sylvain Joyeux <sylvain [dot] joyeux [..] ...> wrote:
> On 01/31/2013 05:44 PM, Peter Soetens wrote:
>>
>> Also, I had a question regarding typegen -i pkgname, where 'pkgname'
>> is also typegen generated:
>> It seems that it picks up the .tlb files, but still the code for the
>> transport of these earlier defined types is generated. Can typegen be
>> changed such that for these -i specified packages, no code is
>> generated anymore ?
>
> It is a typegen-specific issue, orogen does show the "right" behaviour.
>
> I'm having a look and I am puzzled. Could you send the .pc file for the
> imported typekit ?

It seems the culprit is in typegen itself, the check:

pkg.typelist && !pkg.typelist.empty?

is always false. Indeed, there is no typelist entry:

$ cat build/typekit/mockup-typekit-gnulinux.pc
# Generated from orogen/lib/orogen/templates/typekit/typekit.pc

prefix=/usr/local
exec_prefix=${prefix}
libdir=${prefix}/lib/orocos/gnulinux/mockup/types
includedir=${prefix}/include/orocos

project_name=mockup

type_registry=${prefix}/share/orogen/mockup.tlb

Name: mockupTypekit
Version:
Requires: orocos-kdl
Description: mockup types support for the Orocos type system
Libs: -L${libdir} -lmockup-typekit-gnulinux
Cflags: -I${includedir} -I${includedir}/mockup/types
"-DOROCOS_TARGET=gnulinux"
"-I/home/kaltan/src/git/euron/electric/orocos_toolchain/rtt/../install/include"
"-I/opt/ros/fuerte/stacks/orocos_kinematics_dynamics/orocos_kdl/install_dir/include"
"-I/usr/include/eigen3"

Peter