Updates on the 2.2.0 issues

Submitted by peter on Thu, 2011-01-13 21:52

I have pushed some important fixes to 2.2 branch for the property/typekit code
regarding reading/writing properties we discovered in 2.2.0:

- convertType() in typekits should work again as in 2.1.x
- writeProperties() will call storeProperties() if the target file does not
exist - works around writeProperties not doing the right thing when the file is
empty.
- Fix for segfault when cleaning up property bags that contained decomposed
properties.

Other fixes that were done earlier include:
- update port's service documentation when doc() is used

- provide an <rtt/Component.hp

header such that building a component does
not depend on including OCL headers (slightly untested, but should work)

- the export cflags/ldflags have been removed from the manifest.xml file. That's
because they hardcoded 'gnulinux' as a target, which breaks Xenomai builds
when ROS_ROOT is set. It had to go, it's gone. Use the UseOrocos.cmake macros
to get the correct flags back again, both for typekits, plugins or components.

- A second type info object for the same C++ type will be ignored (a warning
will be logged). Earlier, it replaced the former one, leading to segfaults in
some cases. On the master branch (2.3), we'll install a type alias in these
cases such that the script code or property files can work with both typekits.

Next to these 'minor' fixes, I have worked on the properties'
composition/decomposition problem on a separate branch. The code was seriously
reworked and a bunch of unit tests were added to test compose/decompose of
structs and sequences (and sequences of sequences). If the property issues
remain I can merge this branch to the 2.2 line and see if that improves things
for users. It should in any case (no user code should be affected, the API
remains the same) but I prefer to merge it on master than on the stable
branch, if stable is not entirely broken.

I'll soon merge the toolchain-2.2 changes to the master branch, so people
relying on the manifest.xml of that branch will get a cold shower.

Any questions or remarks welcome.

Peter

‹ Deployer adds data to ethernetpackage enum are unknown_t ›
»

Updates on the 2.2.0 issues

Submitted by Polch on Fri, 2011-01-28 17:52.

Hello peter

Quote:
- provide an <rtt/Component.hp

It's a good idea to have included this file here. I will be able to remove some ugly tests in my CMakeFiles.

I have encountered some problems at the compil time with this new file in the 2.2.1 release.

I suggest this patch.

»

Re: Updates on the 2.2.0 issues

Submitted by Polch on Fri, 2011-01-28 17:48.

Hello peter

Quote:
- provide an <rtt/Component.hpp>

It's a good idea to have included this file here. I will be able to remove some ugly tests in my CMakeFiles.

I have encountered some problems at the compil time with this new file in the 2.2.1 release.

I suggest this patch.

»

Updates on the 2.2.0 issues

Submitted by peter on Wed, 2011-01-19 09:28.

On Thursday 13 January 2011 22:47:54 Peter Soetens wrote:
> I have pushed some important fixes to 2.2 branch for the property/typekit
> code regarding reading/writing properties we discovered in 2.2.0:
...
>
> - A second type info object for the same C++ type will be ignored (a
> warning will be logged). Earlier, it replaced the former one, leading to
> segfaults in some cases. On the master branch (2.3), we'll install a type
> alias in these cases such that the script code or property files can work
> with both typekits.

I have reverted this patch on the toolchain-2.2 branch. It's back to old
behavior. We'll need to work out a better solution on the master branch before
2.3 is released.

This solution will have to satisfy these criteria:
- prefer the last registered type
- keep as much as possible working from the old type, ie remember its name and
its transports

Peter

»

Updates on the 2.2.0 issues

Submitted by peter on Wed, 2011-01-19 09:28.

On Thursday 13 January 2011 22:47:54 Peter Soetens wrote:
> I have pushed some important fixes to 2.2 branch for the property/typekit
> code regarding reading/writing properties we discovered in 2.2.0:
...
>
> - A second type info object for the same C++ type will be ignored (a
> warning will be logged). Earlier, it replaced the former one, leading to
> segfaults in some cases. On the master branch (2.3), we'll install a type
> alias in these cases such that the script code or property files can work
> with both typekits.

I have reverted this patch on the toolchain-2.2 branch. It's back to old
behavior. We'll need to work out a better solution on the master branch before
2.3 is released.

This solution will have to satisfy these criteria:
- prefer the last registered type
- keep as much as possible working from the old type, ie remember its name and
its transports

Peter

»

Updates on the 2.2.0 issues

Submitted by Sylvain Joyeux on Fri, 2011-01-14 08:52.

On 01/13/2011 10:47 PM, Peter Soetens wrote:
> - A second type info object for the same C++ type will be ignored (a warning
> will be logged). Earlier, it replaced the former one, leading to segfaults in
> some cases. On the master branch (2.3), we'll install a type alias in these
> cases such that the script code or property files can work with both typekits.
Waaait a minute.

What I saw from some of your patches is that using the *same* TypeInfo
object for two different types is ignored. Did you really remove
overriding typeinfos ?

»

Updates on the 2.2.0 issues

Submitted by peter on Fri, 2011-01-14 10:00.

On Friday 14 January 2011 09:49:46 Sylvain Joyeux wrote:
> On 01/13/2011 10:47 PM, Peter Soetens wrote:
> > - A second type info object for the same C++ type will be ignored (a
> > warning will be logged). Earlier, it replaced the former one, leading to
> > segfaults in some cases. On the master branch (2.3), we'll install a
> > type alias in these cases such that the script code or property files
> > can work with both typekits.
>
> Waaait a minute.

:-)

>
> What I saw from some of your patches is that using the *same* TypeInfo
> object for two different types is ignored. Did you really remove
> overriding typeinfos ?

I'll sketch this more in detail what we saw and what we did:

Both ROS typekit and RTT typekit were loaded in the same process. Both define
the 'vector<double>' C++ type, but with a different RTT typename (RTT uses
"array", ROS uses "float64[]". The 2.2.0 code in TemplateTypeInfo overwrote the
static variable for that type with what was loaded last, and printed a
warning. Both type info objects were registered in the TypeRepository under
two different names. This caused segfaults we found hard to debug, but only
loading one of the two fixed the crashes.

The proposed fix is this:
For TemplateTypeInfo: don't override an existing TypeInfo static variable for
any C++ type X and print a warning that something fishy is going on.
For TypeInfoRepository: in addType, delete the type info object if the type
name is already in use and return false, instead of overwriting the existing
entry and keeping it.

If this breaks your code, I'll revert the patches immediately, but the thing
we *must* solve is that the static DataSourceTypeInfo<T>::TypeInfo variable
can only hold one typeinfo object for each C++ type T. And if the
TypeInfoRepository holds the 'old' type as well, instead of aliasing it to the
new one, there will be a mix of old and new typeinfo objects since some are
acquired by reading DataSourceTypeInfo<T>::TypeInfo and the other through
Types::type("foo"), depending on if the type name came from C++ template
context or from template-less code.

Maybe we should have brought this up first on the list to hear what other
thought about it, but the solution seemed 'conservative' at that time,
meaning, the first one counts.

I wanted to fix this for 2.3 by still keeping the first, but adding the new one
as an alias (only the name, not the type info object) to the first typeinfo
object.

Peter

»

Updates on the 2.2.0 issues

Submitted by Sylvain Joyeux on Fri, 2011-01-14 10:12.

On 01/14/2011 10:56 AM, Peter Soetens wrote:
> Both ROS typekit and RTT typekit were loaded in the same process. Both define
> the 'vector<double>' C++ type, but with a different RTT typename (RTT uses
> "array", ROS uses "float64[]". The 2.2.0 code in TemplateTypeInfo overwrote the
> static variable for that type with what was loaded last, and printed a
> warning. Both type info objects were registered in the TypeRepository under
> two different names. This caused segfaults we found hard to debug, but only
> loading one of the two fixed the crashes.
I have the same situation with orogen with string and
std::vector<double> (which are registered as /std/string and
/std/vector<double>)

orogen components do rely on the fact that they can load the RTT typekit
(that it uses for fundamental types) and overload the complex types with
its own stuff (i.e. std::string, std::vector). This obviously breaks
that assumption, and removes any way I had to overload existing typeinfo.

Bottom line: I could work with the proposed change if the RTT typekit
was split between fundamental types (integers, floats) and complex types
(string, vector, RTT-specific stuff). Which is a split that would make
sense IMO anyway.

> The proposed fix is this:
> For TemplateTypeInfo: don't override an existing TypeInfo static variable for
> any C++ type X and print a warning that something fishy is going on.
> For TypeInfoRepository: in addType, delete the type info object if the type
> name is already in use and return false, instead of overwriting the existing
> entry and keeping it.
>
> If this breaks your code, I'll revert the patches immediately, but the thing
> we *must* solve is that the static DataSourceTypeInfo<T>::TypeInfo variable
> can only hold one typeinfo object for each C++ type T. And if the
> TypeInfoRepository holds the 'old' type as well, instead of aliasing it to the
> new one, there will be a mix of old and new typeinfo objects since some are
> acquired by reading DataSourceTypeInfo<T>::TypeInfo and the other through
> Types::type("foo"), depending on if the type name came from C++ template
> context or from template-less code.
>
> Maybe we should have brought this up first on the list to hear what other
> thought about it, but the solution seemed 'conservative' at that time,
> meaning, the first one counts.
There's nothing conservative about it. The old behaviour was to have the
new TypeInfo, the new one to keep the old one. This changes the typeinfo
handling in fundamental ways.

> I wanted to fix this for 2.3 by still keeping the first, but adding the new one
> as an alias (only the name, not the type info object) to the first typeinfo
> object.

»

Updates on the 2.2.0 issues

Submitted by Sylvain Joyeux on Fri, 2011-01-14 10:40.

On 01/14/2011 11:10 AM, Sylvain Joyeux wrote:
> On 01/14/2011 10:56 AM, Peter Soetens wrote:
>> Both ROS typekit and RTT typekit were loaded in the same process. Both define
>> the 'vector<double>' C++ type, but with a different RTT typename (RTT uses
>> "array", ROS uses "float64[]". The 2.2.0 code in TemplateTypeInfo overwrote the
>> static variable for that type with what was loaded last, and printed a
>> warning. Both type info objects were registered in the TypeRepository under
>> two different names. This caused segfaults we found hard to debug, but only
>> loading one of the two fixed the crashes.
> I have the same situation with orogen with string and
> std::vector<double> (which are registered as /std/string and
> /std/vector<double>)
>
> orogen components do rely on the fact that they can load the RTT typekit
> (that it uses for fundamental types) and overload the complex types with
> its own stuff (i.e. std::string, std::vector). This obviously breaks
> that assumption, and removes any way I had to overload existing typeinfo.
>
> Bottom line: I could work with the proposed change if the RTT typekit
> was split between fundamental types (integers, floats) and complex types
> (string, vector, RTT-specific stuff). Which is a split that would make
> sense IMO anyway.
Thinking about it a bit more. How does that work for transports ? Can I
override transports ?

The TypeInfo part is completely non important for rock ... But
transports are. I guess if I could only override transports that would work.

»

Updates on the 2.2.0 issues

Submitted by peter on Fri, 2011-01-14 11:00.

On Friday 14 January 2011 11:36:10 Sylvain Joyeux wrote:
> On 01/14/2011 11:10 AM, Sylvain Joyeux wrote:
> > On 01/14/2011 10:56 AM, Peter Soetens wrote:
> >> Both ROS typekit and RTT typekit were loaded in the same process. Both
> >> define the 'vector<double>' C++ type, but with a different RTT
> >> typename (RTT uses "array", ROS uses "float64[]". The 2.2.0 code in
> >> TemplateTypeInfo overwrote the static variable for that type with what
> >> was loaded last, and printed a warning. Both type info objects were
> >> registered in the TypeRepository under two different names. This caused
> >> segfaults we found hard to debug, but only loading one of the two fixed
> >> the crashes.
> >
> > I have the same situation with orogen with string and
> > std::vector<double> (which are registered as /std/string and
> > /std/vector<double>)
> >
> > orogen components do rely on the fact that they can load the RTT typekit
> > (that it uses for fundamental types) and overload the complex types with
> > its own stuff (i.e. std::string, std::vector). This obviously breaks
> > that assumption, and removes any way I had to overload existing typeinfo.
> >
> > Bottom line: I could work with the proposed change if the RTT typekit
> > was split between fundamental types (integers, floats) and complex types
> > (string, vector, RTT-specific stuff). Which is a split that would make
> > sense IMO anyway.
>
> Thinking about it a bit more. How does that work for transports ? Can I
> override transports ?
>
> The TypeInfo part is completely non important for rock ... But
> transports are. I guess if I could only override transports that would
> work.

Good question. When we remove the oldest typeinfo object, we also remove its
associated transports (stored in the type info). We could migrate the
transports to the new one, after all it's exactly the same C++ type.

I don't think any code caches the TypeInfo object, as a quick lookup revealed.
The only thing I noticed is that the corba remote channel elements cache their
corba transport class (other transports do probably the same), but that
shouldn't be an issue if we would migrate these objects accordingly to the new
type info object.

Peter

»

Updates on the 2.2.0 issues

Submitted by peter on Fri, 2011-01-14 10:32.

On Friday 14 January 2011 11:10:54 Sylvain Joyeux wrote:
> On 01/14/2011 10:56 AM, Peter Soetens wrote:
> > Both ROS typekit and RTT typekit were loaded in the same process. Both
> > define the 'vector<double>' C++ type, but with a different RTT typename
> > (RTT uses "array", ROS uses "float64[]". The 2.2.0 code in
> > TemplateTypeInfo overwrote the static variable for that type with what
> > was loaded last, and printed a warning. Both type info objects were
> > registered in the TypeRepository under two different names. This caused
> > segfaults we found hard to debug, but only loading one of the two fixed
> > the crashes.
>
> I have the same situation with orogen with string and
> std::vector<double> (which are registered as /std/string and
> /std/vector<double>)
>
> orogen components do rely on the fact that they can load the RTT typekit
> (that it uses for fundamental types) and overload the complex types with
> its own stuff (i.e. std::string, std::vector). This obviously breaks
> that assumption, and removes any way I had to overload existing typeinfo.
>
> Bottom line: I could work with the proposed change if the RTT typekit
> was split between fundamental types (integers, floats) and complex types
> (string, vector, RTT-specific stuff). Which is a split that would make
> sense IMO anyway.

Hmm. So you were using this mechanism and you did not have any problems ? It
could be that by accident the ROS typekit was broken in some way and that our
solution only avoided loading these ROS types, hence excluding the wrong bug.

>
> > The proposed fix is this:
> > For TemplateTypeInfo: don't override an existing TypeInfo static variable
> > for any C++ type X and print a warning that something fishy is going on.
> > For TypeInfoRepository: in addType, delete the type info object if the
> > type name is already in use and return false, instead of overwriting the
> > existing entry and keeping it.
> >
> > If this breaks your code, I'll revert the patches immediately, but the
> > thing we *must* solve is that the static DataSourceTypeInfo<T>::TypeInfo
> > variable can only hold one typeinfo object for each C++ type T. And if
> > the TypeInfoRepository holds the 'old' type as well, instead of aliasing
> > it to the new one, there will be a mix of old and new typeinfo objects
> > since some are acquired by reading DataSourceTypeInfo<T>::TypeInfo and
> > the other through Types::type("foo"), depending on if the type name came
> > from C++ template context or from template-less code.
> >
> > Maybe we should have brought this up first on the list to hear what other
> > thought about it, but the solution seemed 'conservative' at that time,
> > meaning, the first one counts.
>
> There's nothing conservative about it. The old behaviour was to have the
> new TypeInfo, the new one to keep the old one. This changes the typeinfo
> handling in fundamental ways.

Not entirely. The old behavior kept 'both' typeinfo objects in the repository,
but only stored one of them in the global type info variable. Maybe the better
fix on this stable branch is to :

- keep the old overriding behavior in TemplateTypeInfo
- keep *only* the last type info object in the repository, and update the map
such that a lookup for the first gives you the last.
- delete the first type info object

Better/Worse ?

Peter

»

Updates on the 2.2.0 issues

Submitted by markus.klotzbuecher on Wed, 2011-01-26 13:44.

On Fri, Jan 14, 2011 at 11:28:48AM +0100, Peter Soetens wrote:
> On Friday 14 January 2011 11:10:54 Sylvain Joyeux wrote:
> > On 01/14/2011 10:56 AM, Peter Soetens wrote:
> > > Both ROS typekit and RTT typekit were loaded in the same process. Both
> > > define the 'vector<double>' C++ type, but with a different RTT typename
> > > (RTT uses "array", ROS uses "float64[]". The 2.2.0 code in
> > > TemplateTypeInfo overwrote the static variable for that type with what
> > > was loaded last, and printed a warning. Both type info objects were
> > > registered in the TypeRepository under two different names. This caused
> > > segfaults we found hard to debug, but only loading one of the two fixed
> > > the crashes.
> >
> > I have the same situation with orogen with string and
> > std::vector<double> (which are registered as /std/string and
> > /std/vector<double>)
> >
> > orogen components do rely on the fact that they can load the RTT typekit
> > (that it uses for fundamental types) and overload the complex types with
> > its own stuff (i.e. std::string, std::vector). This obviously breaks
> > that assumption, and removes any way I had to overload existing typeinfo.
> >
> > Bottom line: I could work with the proposed change if the RTT typekit
> > was split between fundamental types (integers, floats) and complex types
> > (string, vector, RTT-specific stuff). Which is a split that would make
> > sense IMO anyway.
>
> Hmm. So you were using this mechanism and you did not have any problems ? It
> could be that by accident the ROS typekit was broken in some way and that our
> solution only avoided loading these ROS types, hence excluding the wrong bug.
>
> >
> > > The proposed fix is this:
> > > For TemplateTypeInfo: don't override an existing TypeInfo static variable
> > > for any C++ type X and print a warning that something fishy is going on.
> > > For TypeInfoRepository: in addType, delete the type info object if the
> > > type name is already in use and return false, instead of overwriting the
> > > existing entry and keeping it.
> > >
> > > If this breaks your code, I'll revert the patches immediately, but the
> > > thing we *must* solve is that the static DataSourceTypeInfo<T>::TypeInfo
> > > variable can only hold one typeinfo object for each C++ type T. And if
> > > the TypeInfoRepository holds the 'old' type as well, instead of aliasing
> > > it to the new one, there will be a mix of old and new typeinfo objects
> > > since some are acquired by reading DataSourceTypeInfo<T>::TypeInfo and
> > > the other through Types::type("foo"), depending on if the type name came
> > > from C++ template context or from template-less code.
> > >
> > > Maybe we should have brought this up first on the list to hear what other
> > > thought about it, but the solution seemed 'conservative' at that time,
> > > meaning, the first one counts.
> >
> > There's nothing conservative about it. The old behaviour was to have the
> > new TypeInfo, the new one to keep the old one. This changes the typeinfo
> > handling in fundamental ways.
>
> Not entirely. The old behavior kept 'both' typeinfo objects in the repository,
> but only stored one of them in the global type info variable. Maybe the better
> fix on this stable branch is to :
>
> - keep the old overriding behavior in TemplateTypeInfo
> - keep *only* the last type info object in the repository, and update the map
> such that a lookup for the first gives you the last.
> - delete the first type info object

We just got hit by the overriding. The result is that that the basic
types (int, double, ...) which had meaning before are suddenly
semantic free and non decomposable to lua scripting. Ugly.

So the overriding is causing trouble here, altough I do not claim that
overriding or not is the wrong or right thing to do. A flag to force
overriding or one to prevent overriding would have been helpful.

Where should this be solved? One thing that comes to mind would be to
compare typeinfo pointers instead of names. As you register aliases, I
would assume the the following might work. E.g. to figure out if a
give DSB is actually a double could I do:

TypeInfo* ti_unkown = unknown_dsb->getTypeInfo()
TypeInfo *ti_double = TypeInfoRepository::Instance()->type("double");
 
if(ti_unkown == t2_double)
      /* its a double */
else
	/* its sth else */

This is not very nice.

A better solution might be to extend what we had in mind for
identifying that a type is indexable. For example how about extending
TypeInfo with a method:

TypeCat getTypeCategory()

with enum TypeCat { array, struct, floatingpoint, integer, string, boolean };

That way I would know that a "my_float64_type", being typecat
'floatingpoint' should be convertable to a double.

Any better ideas?

Markus

»

Updates on the 2.2.0 issues

Submitted by peter on Wed, 2011-01-26 14:16.

On Wednesday 26 January 2011 14:41:11 Markus Klotzbuecher wrote:
> On Fri, Jan 14, 2011 at 11:28:48AM +0100, Peter Soetens wrote:
> > On Friday 14 January 2011 11:10:54 Sylvain Joyeux wrote:
> > > On 01/14/2011 10:56 AM, Peter Soetens wrote:
> > > > Both ROS typekit and RTT typekit were loaded in the same process.
> > > > Both define the 'vector<double>' C++ type, but with a different RTT
> > > > typename (RTT uses "array", ROS uses "float64[]". The 2.2.0 code in
> > > > TemplateTypeInfo overwrote the static variable for that type with
> > > > what was loaded last, and printed a warning. Both type info objects
> > > > were registered in the TypeRepository under two different names.
> > > > This caused segfaults we found hard to debug, but only loading one
> > > > of the two fixed the crashes.
> > >
> > > I have the same situation with orogen with string and
> > > std::vector<double> (which are registered as /std/string and
> > > /std/vector<double>)
> > >
> > > orogen components do rely on the fact that they can load the RTT
> > > typekit (that it uses for fundamental types) and overload the complex
> > > types with its own stuff (i.e. std::string, std::vector). This
> > > obviously breaks that assumption, and removes any way I had to
> > > overload existing typeinfo.
> > >
> > > Bottom line: I could work with the proposed change if the RTT typekit
> > > was split between fundamental types (integers, floats) and complex
> > > types (string, vector, RTT-specific stuff). Which is a split that
> > > would make sense IMO anyway.
> >
> > Hmm. So you were using this mechanism and you did not have any problems ?
> > It could be that by accident the ROS typekit was broken in some way and
> > that our solution only avoided loading these ROS types, hence excluding
> > the wrong bug.
> >
> > > > The proposed fix is this:
> > > > For TemplateTypeInfo: don't override an existing TypeInfo static
> > > > variable for any C++ type X and print a warning that something fishy
> > > > is going on. For TypeInfoRepository: in addType, delete the type
> > > > info object if the type name is already in use and return false,
> > > > instead of overwriting the existing entry and keeping it.
> > > >
> > > > If this breaks your code, I'll revert the patches immediately, but
> > > > the thing we *must* solve is that the static
> > > > DataSourceTypeInfo<T>::TypeInfo variable can only hold one typeinfo
> > > > object for each C++ type T. And if the TypeInfoRepository holds the
> > > > 'old' type as well, instead of aliasing it to the new one, there
> > > > will be a mix of old and new typeinfo objects since some are
> > > > acquired by reading DataSourceTypeInfo<T>::TypeInfo and the other
> > > > through Types::type("foo"), depending on if the type name came from
> > > > C++ template context or from template-less code.
> > > >
> > > > Maybe we should have brought this up first on the list to hear what
> > > > other thought about it, but the solution seemed 'conservative' at
> > > > that time, meaning, the first one counts.
> > >
> > > There's nothing conservative about it. The old behaviour was to have
> > > the new TypeInfo, the new one to keep the old one. This changes the
> > > typeinfo handling in fundamental ways.
> >
> > Not entirely. The old behavior kept 'both' typeinfo objects in the
> > repository, but only stored one of them in the global type info
> > variable. Maybe the better
> >
> > fix on this stable branch is to :
> > - keep the old overriding behavior in TemplateTypeInfo
> > - keep *only* the last type info object in the repository, and update
> > the map
> >
> > such that a lookup for the first gives you the last.
> >
> > - delete the first type info object
>
> We just got hit by the overriding. The result is that that the basic
> types (int, double, ...) which had meaning before are suddenly
> semantic free and non decomposable to lua scripting. Ugly.

?? Could you elaborate on this ?

>
> So the overriding is causing trouble here, altough I do not claim that
> overriding or not is the wrong or right thing to do. A flag to force
> overriding or one to prevent overriding would have been helpful.
>
> Where should this be solved? One thing that comes to mind would be to
> compare typeinfo pointers instead of names. As you register aliases, I
> would assume the the following might work. E.g. to figure out if a
> give DSB is actually a double could I do:
>
> 

> TypeInfo* ti_unkown = unknown_dsb->getTypeInfo()
> TypeInfo *ti_double = TypeInfoRepository::Instance()->type("double");
> 
> if(ti_unkown == t2_double)
>       /* its a double */
> else
> 	/* its sth else */
>

>
> This is not very nice.
>
> A better solution might be to extend what we had in mind for
> identifying that a type is indexable. For example how about extending
> TypeInfo with a method:
>
> TypeCat getTypeCategory()
>
> with enum TypeCat { array, struct, floatingpoint, integer, string, boolean
> };
>
> That way I would know that a "my_float64_type", being typecat
> 'floatingpoint' should be convertable to a double.
>
> Any better ideas?

I'm open to suggestions myself.

Peter

»

Updates on the 2.2.0 issues

Submitted by Sylvain Joyeux on Wed, 2011-01-26 15:52.

On 01/26/2011 03:11 PM, Peter Soetens wrote:
> On Wednesday 26 January 2011 14:41:11 Markus Klotzbuecher wrote:
>> On Fri, Jan 14, 2011 at 11:28:48AM +0100, Peter Soetens wrote:
>>> On Friday 14 January 2011 11:10:54 Sylvain Joyeux wrote:
>>>> On 01/14/2011 10:56 AM, Peter Soetens wrote:
>>>>> Both ROS typekit and RTT typekit were loaded in the same process.
>>>>> Both define the 'vector<double>' C++ type, but with a different RTT
>>>>> typename (RTT uses "array", ROS uses "float64[]". The 2.2.0 code in
>>>>> TemplateTypeInfo overwrote the static variable for that type with
>>>>> what was loaded last, and printed a warning. Both type info objects
>>>>> were registered in the TypeRepository under two different names.
>>>>> This caused segfaults we found hard to debug, but only loading one
>>>>> of the two fixed the crashes.
>>>>
>>>> I have the same situation with orogen with string and
>>>> std::vector<double> (which are registered as /std/string and
>>>> /std/vector<double>)
>>>>
>>>> orogen components do rely on the fact that they can load the RTT
>>>> typekit (that it uses for fundamental types) and overload the complex
>>>> types with its own stuff (i.e. std::string, std::vector). This
>>>> obviously breaks that assumption, and removes any way I had to
>>>> overload existing typeinfo.
>>>>
>>>> Bottom line: I could work with the proposed change if the RTT typekit
>>>> was split between fundamental types (integers, floats) and complex
>>>> types (string, vector, RTT-specific stuff). Which is a split that
>>>> would make sense IMO anyway.
>>>
>>> Hmm. So you were using this mechanism and you did not have any problems ?
>>> It could be that by accident the ROS typekit was broken in some way and
>>> that our solution only avoided loading these ROS types, hence excluding
>>> the wrong bug.
>>>
>>>>> The proposed fix is this:
>>>>> For TemplateTypeInfo: don't override an existing TypeInfo static
>>>>> variable for any C++ type X and print a warning that something fishy
>>>>> is going on. For TypeInfoRepository: in addType, delete the type
>>>>> info object if the type name is already in use and return false,
>>>>> instead of overwriting the existing entry and keeping it.
>>>>>
>>>>> If this breaks your code, I'll revert the patches immediately, but
>>>>> the thing we *must* solve is that the static
>>>>> DataSourceTypeInfo<T>::TypeInfo variable can only hold one typeinfo
>>>>> object for each C++ type T. And if the TypeInfoRepository holds the
>>>>> 'old' type as well, instead of aliasing it to the new one, there
>>>>> will be a mix of old and new typeinfo objects since some are
>>>>> acquired by reading DataSourceTypeInfo<T>::TypeInfo and the other
>>>>> through Types::type("foo"), depending on if the type name came from
>>>>> C++ template context or from template-less code.
>>>>>
>>>>> Maybe we should have brought this up first on the list to hear what
>>>>> other thought about it, but the solution seemed 'conservative' at
>>>>> that time, meaning, the first one counts.
>>>>
>>>> There's nothing conservative about it. The old behaviour was to have
>>>> the new TypeInfo, the new one to keep the old one. This changes the
>>>> typeinfo handling in fundamental ways.
>>>
>>> Not entirely. The old behavior kept 'both' typeinfo objects in the
>>> repository, but only stored one of them in the global type info
>>> variable. Maybe the better
>>>
>>> fix on this stable branch is to :
>>> - keep the old overriding behavior in TemplateTypeInfo
>>> - keep *only* the last type info object in the repository, and update
>>> the map
>>>
>>> such that a lookup for the first gives you the last.
>>>
>>> - delete the first type info object
>>
>> We just got hit by the overriding. The result is that that the basic
>> types (int, double, ...) which had meaning before are suddenly
>> semantic free and non decomposable to lua scripting. Ugly.
>
> ?? Could you elaborate on this ?
>
>>
>> So the overriding is causing trouble here, altough I do not claim that
>> overriding or not is the wrong or right thing to do. A flag to force
>> overriding or one to prevent overriding would have been helpful.
>>
>> Where should this be solved? One thing that comes to mind would be to
>> compare typeinfo pointers instead of names. As you register aliases, I
>> would assume the the following might work. E.g. to figure out if a
>> give DSB is actually a double could I do:
>>
>> 

>> TypeInfo* ti_unkown = unknown_dsb->getTypeInfo()
>> TypeInfo *ti_double = TypeInfoRepository::Instance()->type("double");
>>
>> if(ti_unkown == t2_double)
>>        /* its a double */
>> else
>> 	/* its sth else */
>>

>>
>> This is not very nice.
>>
>> A better solution might be to extend what we had in mind for
>> identifying that a type is indexable. For example how about extending
>> TypeInfo with a method:
>>
>> TypeCat getTypeCategory()
>>
>> with enum TypeCat { array, struct, floatingpoint, integer, string, boolean
>> };
>>
>> That way I would know that a "my_float64_type", being typecat
>> 'floatingpoint' should be convertable to a double.
>>
>> Any better ideas?
>
> I'm open to suggestions myself.
I know I'm already annoying with this. But my suggestion is: use typelib.

No kidding.

There is already a full featured type system that allows to access C/C++
data structures from a scripting language (in my case: Ruby). The thing
you are currently trying to do in the RTT type system is to recreate it,
and this is painful to watch.

»
Ruben Smits's picture

Updates on the 2.2.0 issues

Submitted by Ruben Smits on Wed, 2011-01-26 20:52.

On Wednesday 26 January 2011 16:48:56 Sylvain Joyeux wrote:
> On 01/26/2011 03:11 PM, Peter Soetens wrote:
> > On Wednesday 26 January 2011 14:41:11 Markus Klotzbuecher wrote:
> >> On Fri, Jan 14, 2011 at 11:28:48AM +0100, Peter Soetens wrote:
> >>> On Friday 14 January 2011 11:10:54 Sylvain Joyeux wrote:
> >>>> On 01/14/2011 10:56 AM, Peter Soetens wrote:
> >>>>> Both ROS typekit and RTT typekit were loaded in the same
> >>>>> process.
> >>>>> Both define the 'vector<double>' C++ type, but with a different
> >>>>> RTT
> >>>>> typename (RTT uses "array", ROS uses "float64[]". The 2.2.0 code
> >>>>> in
> >>>>> TemplateTypeInfo overwrote the static variable for that type
> >>>>> with
> >>>>> what was loaded last, and printed a warning. Both type info
> >>>>> objects
> >>>>> were registered in the TypeRepository under two different names.
> >>>>> This caused segfaults we found hard to debug, but only loading
> >>>>> one
> >>>>> of the two fixed the crashes.
> >>>>
> >>>> I have the same situation with orogen with string and
> >>>> std::vector<double> (which are registered as /std/string and
> >>>> /std/vector<double>)
> >>>>
> >>>> orogen components do rely on the fact that they can load the RTT
> >>>> typekit (that it uses for fundamental types) and overload the
> >>>> complex
> >>>> types with its own stuff (i.e. std::string, std::vector). This
> >>>> obviously breaks that assumption, and removes any way I had to
> >>>> overload existing typeinfo.
> >>>>
> >>>> Bottom line: I could work with the proposed change if the RTT
> >>>> typekit
> >>>> was split between fundamental types (integers, floats) and complex
> >>>> types (string, vector, RTT-specific stuff). Which is a split that
> >>>> would make sense IMO anyway.
> >>>
> >>> Hmm. So you were using this mechanism and you did not have any
> >>> problems ? It could be that by accident the ROS typekit was broken
> >>> in some way and that our solution only avoided loading these ROS
> >>> types, hence excluding the wrong bug.
> >>>
> >>>>> The proposed fix is this:
> >>>>> For TemplateTypeInfo: don't override an existing TypeInfo static
> >>>>> variable for any C++ type X and print a warning that something
> >>>>> fishy
> >>>>> is going on. For TypeInfoRepository: in addType, delete the type
> >>>>> info object if the type name is already in use and return false,
> >>>>> instead of overwriting the existing entry and keeping it.
> >>>>>
> >>>>> If this breaks your code, I'll revert the patches immediately,
> >>>>> but
> >>>>> the thing we *must* solve is that the static
> >>>>> DataSourceTypeInfo<T>::TypeInfo variable can only hold one
> >>>>> typeinfo
> >>>>> object for each C++ type T. And if the TypeInfoRepository holds
> >>>>> the
> >>>>> 'old' type as well, instead of aliasing it to the new one, there
> >>>>> will be a mix of old and new typeinfo objects since some are
> >>>>> acquired by reading DataSourceTypeInfo<T>::TypeInfo and the
> >>>>> other
> >>>>> through Types::type("foo"), depending on if the type name came
> >>>>> from
> >>>>> C++ template context or from template-less code.
> >>>>>
> >>>>> Maybe we should have brought this up first on the list to hear
> >>>>> what
> >>>>> other thought about it, but the solution seemed 'conservative'
> >>>>> at
> >>>>> that time, meaning, the first one counts.
> >>>>
> >>>> There's nothing conservative about it. The old behaviour was to
> >>>> have
> >>>> the new TypeInfo, the new one to keep the old one. This changes
> >>>> the
> >>>> typeinfo handling in fundamental ways.
> >>>
> >>> Not entirely. The old behavior kept 'both' typeinfo objects in the
> >>> repository, but only stored one of them in the global type info
> >>> variable. Maybe the better
> >>>
> >>> fix on this stable branch is to :
> >>> - keep the old overriding behavior in TemplateTypeInfo
> >>> - keep *only* the last type info object in the repository, and
> >>> update
> >>> the map
> >>>
> >>> such that a lookup for the first gives you the last.
> >>>
> >>> - delete the first type info object
> >>
> >> We just got hit by the overriding. The result is that that the basic
> >> types (int, double, ...) which had meaning before are suddenly
> >> semantic free and non decomposable to lua scripting. Ugly.
> >
> > ?? Could you elaborate on this ?
> >
> >> So the overriding is causing trouble here, altough I do not claim that
> >> overriding or not is the wrong or right thing to do. A flag to force
> >> overriding or one to prevent overriding would have been helpful.
> >>
> >> Where should this be solved? One thing that comes to mind would be to
> >> compare typeinfo pointers instead of names. As you register aliases, I
> >> would assume the the following might work. E.g. to figure out if a
> >> give DSB is actually a double could I do:
> >>
> >> 

> >> TypeInfo* ti_unkown = unknown_dsb->getTypeInfo()
> >> TypeInfo *ti_double = TypeInfoRepository::Instance()->type("double");
> >> 
> >> if(ti_unkown == t2_double)
> >> 
> >>        /* its a double */
> >> 
> >> else
> >> 
> >> 	/* its sth else */
> >> 
> >>

> >>
> >> This is not very nice.
> >>
> >> A better solution might be to extend what we had in mind for
> >> identifying that a type is indexable. For example how about extending
> >> TypeInfo with a method:
> >>
> >> TypeCat getTypeCategory()
> >>
> >> with enum TypeCat { array, struct, floatingpoint, integer, string,
> >> boolean };
> >>
> >> That way I would know that a "my_float64_type", being typecat
> >> 'floatingpoint' should be convertable to a double.
> >>
> >> Any better ideas?
> >
> > I'm open to suggestions myself.
>
> I know I'm already annoying with this. But my suggestion is: use typelib.
>
> No kidding.
>
> There is already a full featured type system that allows to access C/C++
> data structures from a scripting language (in my case: Ruby). The thing
> you are currently trying to do in the RTT type system is to recreate it,
> and this is painful to watch.

Can you point me to documentation on how to use typelib? I have been searching
google (not really helpfull since the name typelib seems to be really
generic), the orocos website (no documentation on typelib itself) and the rock
website (talks about typegen but is not really explaining how it can be used).

Ruben

»

Updates on the 2.2.0 issues

Submitted by Sylvain Joyeux on Thu, 2011-01-27 10:04.

On 01/26/2011 09:48 PM, Ruben Smits wrote:
> Can you point me to documentation on how to use typelib? I have been searching
> google (not really helpfull since the name typelib seems to be really
> generic), the orocos website (no documentation on typelib itself) and the rock
> website (talks about typegen but is not really explaining how it can be used).

There's only API documentation. Click "API documentation" at

http://rock-robotics.org/package_directory/packages/typelib/index.html

I don't really have time right now to explain the details. I'll do a
quick overview:

Typelib has two sides: type representation and value manipulation

On the type representation side (main classes: Typelib::Registry,
subclasses of Typelib::Type and Typelib::TypeVisitor), you represent
your types and their relationships. The type model API is designed to
manipulate C/C++ values (i.e. there are byte sizes and offsets), but
they can safely be ignored if you only use the type representation.

*If* what you manipulate are not "opaques", i.e. *if* typelib can
represent them *including* byte sizes and offsets (in other words: if
typegen can understand them), you have a value manipulation API
(Typelib::Value, Typelib::ValueVisitor, and most of the functions
defined in the Typelib namespace). I.e. you can create, manipulate and
destroy the C/C++ values directly from the Typelib API without
marshalling/demarshalling or other convertions.

Note that I already implemented mechanisms to handle types that typegen
cannot understand, the so-called "opaque" mechanism. This works pretty
well and is IMO still much better than writing anything by hand. But YMMV.

Since you guys seem to want to write typekits by hand (did I already say
"yuk" ?), the most relevant part for you is the type representation.
However, there could be a lot of fastpaths involved in bindings to Lua
that would use typelib directly on types that can be manipulated by typelib.

»

Updates on the 2.2.0 issues

Submitted by markus.klotzbuecher on Thu, 2011-01-27 12:40.

On Thu, Jan 27, 2011 at 11:00:17AM +0100, Sylvain Joyeux wrote:
> On 01/26/2011 09:48 PM, Ruben Smits wrote:
> > Can you point me to documentation on how to use typelib? I have been searching
> > google (not really helpfull since the name typelib seems to be really
> > generic), the orocos website (no documentation on typelib itself) and the rock
> > website (talks about typegen but is not really explaining how it can be used).
>
> There's only API documentation. Click "API documentation" at
>
> http://rock-robotics.org/package_directory/packages/typelib/index.html
>
> I don't really have time right now to explain the details. I'll do a
> quick overview:
>
> Typelib has two sides: type representation and value manipulation
>
> On the type representation side (main classes: Typelib::Registry,
> subclasses of Typelib::Type and Typelib::TypeVisitor), you represent
> your types and their relationships. The type model API is designed to
> manipulate C/C++ values (i.e. there are byte sizes and offsets), but
> they can safely be ignored if you only use the type representation.
>
> *If* what you manipulate are not "opaques", i.e. *if* typelib can
> represent them *including* byte sizes and offsets (in other words: if
> typegen can understand them), you have a value manipulation API
> (Typelib::Value, Typelib::ValueVisitor, and most of the functions
> defined in the Typelib namespace). I.e. you can create, manipulate and
> destroy the C/C++ values directly from the Typelib API without
> marshalling/demarshalling or other convertions.
>
> Note that I already implemented mechanisms to handle types that typegen
> cannot understand, the so-called "opaque" mechanism. This works pretty
> well and is IMO still much better than writing anything by hand. But YMMV.
>
> Since you guys seem to want to write typekits by hand (did I already say
> "yuk" ?), the most relevant part for you is the type representation.
> However, there could be a lot of fastpaths involved in bindings to Lua
> that would use typelib directly on types that can be manipulated by
> typelib.

Hmmm. Could you be more specific? If I have a DataSourcBase with a
unkown type name, how can typelib opaque mechanism help me?

Indeed having (a subset) of Typelib::Type's interface available on a
DSB would solve my problem (especially getCategory is exactly what I
have in mind), but that would require some deeper changes, AFAIK.

Markus

»

Updates on the 2.2.0 issues

Submitted by markus.klotzbuecher on Wed, 2011-01-26 19:32.

On Wed, Jan 26, 2011 at 04:48:56PM +0100, Sylvain Joyeux wrote:
> On 01/26/2011 03:11 PM, Peter Soetens wrote:
> > On Wednesday 26 January 2011 14:41:11 Markus Klotzbuecher wrote:
> >> On Fri, Jan 14, 2011 at 11:28:48AM +0100, Peter Soetens wrote:
> >>> On Friday 14 January 2011 11:10:54 Sylvain Joyeux wrote:
> >>>> On 01/14/2011 10:56 AM, Peter Soetens wrote:
> >>>>> Both ROS typekit and RTT typekit were loaded in the same process.
> >>>>> Both define the 'vector<double>' C++ type, but with a different RTT
> >>>>> typename (RTT uses "array", ROS uses "float64[]". The 2.2.0 code in
> >>>>> TemplateTypeInfo overwrote the static variable for that type with
> >>>>> what was loaded last, and printed a warning. Both type info objects
> >>>>> were registered in the TypeRepository under two different names.
> >>>>> This caused segfaults we found hard to debug, but only loading one
> >>>>> of the two fixed the crashes.
> >>>>
> >>>> I have the same situation with orogen with string and
> >>>> std::vector<double> (which are registered as /std/string and
> >>>> /std/vector<double>)
> >>>>
> >>>> orogen components do rely on the fact that they can load the RTT
> >>>> typekit (that it uses for fundamental types) and overload the complex
> >>>> types with its own stuff (i.e. std::string, std::vector). This
> >>>> obviously breaks that assumption, and removes any way I had to
> >>>> overload existing typeinfo.
> >>>>
> >>>> Bottom line: I could work with the proposed change if the RTT typekit
> >>>> was split between fundamental types (integers, floats) and complex
> >>>> types (string, vector, RTT-specific stuff). Which is a split that
> >>>> would make sense IMO anyway.
> >>>
> >>> Hmm. So you were using this mechanism and you did not have any problems ?
> >>> It could be that by accident the ROS typekit was broken in some way and
> >>> that our solution only avoided loading these ROS types, hence excluding
> >>> the wrong bug.
> >>>
> >>>>> The proposed fix is this:
> >>>>> For TemplateTypeInfo: don't override an existing TypeInfo static
> >>>>> variable for any C++ type X and print a warning that something fishy
> >>>>> is going on. For TypeInfoRepository: in addType, delete the type
> >>>>> info object if the type name is already in use and return false,
> >>>>> instead of overwriting the existing entry and keeping it.
> >>>>>
> >>>>> If this breaks your code, I'll revert the patches immediately, but
> >>>>> the thing we *must* solve is that the static
> >>>>> DataSourceTypeInfo<T>::TypeInfo variable can only hold one typeinfo
> >>>>> object for each C++ type T. And if the TypeInfoRepository holds the
> >>>>> 'old' type as well, instead of aliasing it to the new one, there
> >>>>> will be a mix of old and new typeinfo objects since some are
> >>>>> acquired by reading DataSourceTypeInfo<T>::TypeInfo and the other
> >>>>> through Types::type("foo"), depending on if the type name came from
> >>>>> C++ template context or from template-less code.
> >>>>>
> >>>>> Maybe we should have brought this up first on the list to hear what
> >>>>> other thought about it, but the solution seemed 'conservative' at
> >>>>> that time, meaning, the first one counts.
> >>>>
> >>>> There's nothing conservative about it. The old behaviour was to have
> >>>> the new TypeInfo, the new one to keep the old one. This changes the
> >>>> typeinfo handling in fundamental ways.
> >>>
> >>> Not entirely. The old behavior kept 'both' typeinfo objects in the
> >>> repository, but only stored one of them in the global type info
> >>> variable. Maybe the better
> >>>
> >>> fix on this stable branch is to :
> >>> - keep the old overriding behavior in TemplateTypeInfo
> >>> - keep *only* the last type info object in the repository, and update
> >>> the map
> >>>
> >>> such that a lookup for the first gives you the last.
> >>>
> >>> - delete the first type info object
> >>
> >> We just got hit by the overriding. The result is that that the basic
> >> types (int, double, ...) which had meaning before are suddenly
> >> semantic free and non decomposable to lua scripting. Ugly.
> >
> > ?? Could you elaborate on this ?
> >
> >>
> >> So the overriding is causing trouble here, altough I do not claim that
> >> overriding or not is the wrong or right thing to do. A flag to force
> >> overriding or one to prevent overriding would have been helpful.
> >>
> >> Where should this be solved? One thing that comes to mind would be to
> >> compare typeinfo pointers instead of names. As you register aliases, I
> >> would assume the the following might work. E.g. to figure out if a
> >> give DSB is actually a double could I do:
> >>
> >> 

> >> TypeInfo* ti_unkown = unknown_dsb->getTypeInfo()
> >> TypeInfo *ti_double = TypeInfoRepository::Instance()->type("double");
> >>
> >> if(ti_unkown == t2_double)
> >>        /* its a double */
> >> else
> >> 	/* its sth else */
> >>

> >>
> >> This is not very nice.
> >>
> >> A better solution might be to extend what we had in mind for
> >> identifying that a type is indexable. For example how about extending
> >> TypeInfo with a method:
> >>
> >> TypeCat getTypeCategory()
> >>
> >> with enum TypeCat { array, struct, floatingpoint, integer, string, boolean
> >> };
> >>
> >> That way I would know that a "my_float64_type", being typecat
> >> 'floatingpoint' should be convertable to a double.
> >>
> >> Any better ideas?
> >
> > I'm open to suggestions myself.
> I know I'm already annoying with this. But my suggestion is: use typelib.
>
> No kidding.
>
> There is already a full featured type system that allows to access C/C++
> data structures from a scripting language (in my case: Ruby). The thing
> you are currently trying to do in the RTT type system is to recreate it,
> and this is painful to watch.

Could you explain with a bit more detail how this could work? I'm not
against using typelib, my only requirement is that users need not need
to do any extra work once they have a working typekit.

Markus

»

Updates on the 2.2.0 issues

Submitted by snrkiwi on Wed, 2011-01-26 16:04.

On Jan 26, 2011, at 10:48 , Sylvain Joyeux wrote:

> On 01/26/2011 03:11 PM, Peter Soetens wrote:
>> On Wednesday 26 January 2011 14:41:11 Markus Klotzbuecher wrote:
>>> On Fri, Jan 14, 2011 at 11:28:48AM +0100, Peter Soetens wrote:
>>>> On Friday 14 January 2011 11:10:54 Sylvain Joyeux wrote:
>>>>> On 01/14/2011 10:56 AM, Peter Soetens wrote:
>>>>>> Both ROS typekit and RTT typekit were loaded in the same process.
>>>>>> Both define the 'vector<double>' C++ type, but with a different RTT
>>>>>> typename (RTT uses "array", ROS uses "float64[]". The 2.2.0 code in
>>>>>> TemplateTypeInfo overwrote the static variable for that type with
>>>>>> what was loaded last, and printed a warning. Both type info objects
>>>>>> were registered in the TypeRepository under two different names.
>>>>>> This caused segfaults we found hard to debug, but only loading one
>>>>>> of the two fixed the crashes.
>>>>>
>>>>> I have the same situation with orogen with string and
>>>>> std::vector<double> (which are registered as /std/string and
>>>>> /std/vector<double>)
>>>>>
>>>>> orogen components do rely on the fact that they can load the RTT
>>>>> typekit (that it uses for fundamental types) and overload the complex
>>>>> types with its own stuff (i.e. std::string, std::vector). This
>>>>> obviously breaks that assumption, and removes any way I had to
>>>>> overload existing typeinfo.
>>>>>
>>>>> Bottom line: I could work with the proposed change if the RTT typekit
>>>>> was split between fundamental types (integers, floats) and complex
>>>>> types (string, vector, RTT-specific stuff). Which is a split that
>>>>> would make sense IMO anyway.
>>>>
>>>> Hmm. So you were using this mechanism and you did not have any problems ?
>>>> It could be that by accident the ROS typekit was broken in some way and
>>>> that our solution only avoided loading these ROS types, hence excluding
>>>> the wrong bug.
>>>>
>>>>>> The proposed fix is this:
>>>>>> For TemplateTypeInfo: don't override an existing TypeInfo static
>>>>>> variable for any C++ type X and print a warning that something fishy
>>>>>> is going on. For TypeInfoRepository: in addType, delete the type
>>>>>> info object if the type name is already in use and return false,
>>>>>> instead of overwriting the existing entry and keeping it.
>>>>>>
>>>>>> If this breaks your code, I'll revert the patches immediately, but
>>>>>> the thing we *must* solve is that the static
>>>>>> DataSourceTypeInfo<T>::TypeInfo variable can only hold one typeinfo
>>>>>> object for each C++ type T. And if the TypeInfoRepository holds the
>>>>>> 'old' type as well, instead of aliasing it to the new one, there
>>>>>> will be a mix of old and new typeinfo objects since some are
>>>>>> acquired by reading DataSourceTypeInfo<T>::TypeInfo and the other
>>>>>> through Types::type("foo"), depending on if the type name came from
>>>>>> C++ template context or from template-less code.
>>>>>>
>>>>>> Maybe we should have brought this up first on the list to hear what
>>>>>> other thought about it, but the solution seemed 'conservative' at
>>>>>> that time, meaning, the first one counts.
>>>>>
>>>>> There's nothing conservative about it. The old behaviour was to have
>>>>> the new TypeInfo, the new one to keep the old one. This changes the
>>>>> typeinfo handling in fundamental ways.
>>>>
>>>> Not entirely. The old behavior kept 'both' typeinfo objects in the
>>>> repository, but only stored one of them in the global type info
>>>> variable. Maybe the better
>>>>
>>>> fix on this stable branch is to :
>>>> - keep the old overriding behavior in TemplateTypeInfo
>>>> - keep *only* the last type info object in the repository, and update
>>>> the map
>>>>
>>>> such that a lookup for the first gives you the last.
>>>>
>>>> - delete the first type info object
>>>
>>> We just got hit by the overriding. The result is that that the basic
>>> types (int, double, ...) which had meaning before are suddenly
>>> semantic free and non decomposable to lua scripting. Ugly.
>>
>> ?? Could you elaborate on this ?
>>
>>>
>>> So the overriding is causing trouble here, altough I do not claim that
>>> overriding or not is the wrong or right thing to do. A flag to force
>>> overriding or one to prevent overriding would have been helpful.
>>>
>>> Where should this be solved? One thing that comes to mind would be to
>>> compare typeinfo pointers instead of names. As you register aliases, I
>>> would assume the the following might work. E.g. to figure out if a
>>> give DSB is actually a double could I do:
>>>
>>> 

>>> TypeInfo* ti_unkown = unknown_dsb->getTypeInfo()
>>> TypeInfo *ti_double = TypeInfoRepository::Instance()->type("double");
>>> 
>>> if(ti_unkown == t2_double)
>>>       /* its a double */
>>> else
>>> 	/* its sth else */
>>>

>>>
>>> This is not very nice.
>>>
>>> A better solution might be to extend what we had in mind for
>>> identifying that a type is indexable. For example how about extending
>>> TypeInfo with a method:
>>>
>>> TypeCat getTypeCategory()
>>>
>>> with enum TypeCat { array, struct, floatingpoint, integer, string, boolean
>>> };
>>>
>>> That way I would know that a "my_float64_type", being typecat
>>> 'floatingpoint' should be convertable to a double.
>>>
>>> Any better ideas?
>>
>> I'm open to suggestions myself.
> I know I'm already annoying with this. But my suggestion is: use typelib.
>
> No kidding.
>
> There is already a full featured type system that allows to access C/C++
> data structures from a scripting language (in my case: Ruby). The thing
> you are currently trying to do in the RTT type system is to recreate it,
> and this is painful to watch.

It does seem like a lot of recent ML traffic has been type related. Can someone outline the pro's and con's of the current system vs typelib, for those of us not in the middle of all this?
S

»

Updates on the 2.2.0 issues

Submitted by markus.klotzbuecher on Wed, 2011-01-26 15:00.

On Wed, Jan 26, 2011 at 03:11:52PM +0100, Peter Soetens wrote:
> On Wednesday 26 January 2011 14:41:11 Markus Klotzbuecher wrote:
> > On Fri, Jan 14, 2011 at 11:28:48AM +0100, Peter Soetens wrote:
> > > On Friday 14 January 2011 11:10:54 Sylvain Joyeux wrote:
> > > > On 01/14/2011 10:56 AM, Peter Soetens wrote:
> > > > > Both ROS typekit and RTT typekit were loaded in the same process.
> > > > > Both define the 'vector<double>' C++ type, but with a different RTT
> > > > > typename (RTT uses "array", ROS uses "float64[]". The 2.2.0 code in
> > > > > TemplateTypeInfo overwrote the static variable for that type with
> > > > > what was loaded last, and printed a warning. Both type info objects
> > > > > were registered in the TypeRepository under two different names.
> > > > > This caused segfaults we found hard to debug, but only loading one
> > > > > of the two fixed the crashes.
> > > >
> > > > I have the same situation with orogen with string and
> > > > std::vector<double> (which are registered as /std/string and
> > > > /std/vector<double>)
> > > >
> > > > orogen components do rely on the fact that they can load the RTT
> > > > typekit (that it uses for fundamental types) and overload the complex
> > > > types with its own stuff (i.e. std::string, std::vector). This
> > > > obviously breaks that assumption, and removes any way I had to
> > > > overload existing typeinfo.
> > > >
> > > > Bottom line: I could work with the proposed change if the RTT typekit
> > > > was split between fundamental types (integers, floats) and complex
> > > > types (string, vector, RTT-specific stuff). Which is a split that
> > > > would make sense IMO anyway.
> > >
> > > Hmm. So you were using this mechanism and you did not have any problems ?
> > > It could be that by accident the ROS typekit was broken in some way and
> > > that our solution only avoided loading these ROS types, hence excluding
> > > the wrong bug.
> > >
> > > > > The proposed fix is this:
> > > > > For TemplateTypeInfo: don't override an existing TypeInfo static
> > > > > variable for any C++ type X and print a warning that something fishy
> > > > > is going on. For TypeInfoRepository: in addType, delete the type
> > > > > info object if the type name is already in use and return false,
> > > > > instead of overwriting the existing entry and keeping it.
> > > > >
> > > > > If this breaks your code, I'll revert the patches immediately, but
> > > > > the thing we *must* solve is that the static
> > > > > DataSourceTypeInfo<T>::TypeInfo variable can only hold one typeinfo
> > > > > object for each C++ type T. And if the TypeInfoRepository holds the
> > > > > 'old' type as well, instead of aliasing it to the new one, there
> > > > > will be a mix of old and new typeinfo objects since some are
> > > > > acquired by reading DataSourceTypeInfo<T>::TypeInfo and the other
> > > > > through Types::type("foo"), depending on if the type name came from
> > > > > C++ template context or from template-less code.
> > > > >
> > > > > Maybe we should have brought this up first on the list to hear what
> > > > > other thought about it, but the solution seemed 'conservative' at
> > > > > that time, meaning, the first one counts.
> > > >
> > > > There's nothing conservative about it. The old behaviour was to have
> > > > the new TypeInfo, the new one to keep the old one. This changes the
> > > > typeinfo handling in fundamental ways.
> > >
> > > Not entirely. The old behavior kept 'both' typeinfo objects in the
> > > repository, but only stored one of them in the global type info
> > > variable. Maybe the better
> > >
> > > fix on this stable branch is to :
> > > - keep the old overriding behavior in TemplateTypeInfo
> > > - keep *only* the last type info object in the repository, and update
> > > the map
> > >
> > > such that a lookup for the first gives you the last.
> > >
> > > - delete the first type info object
> >
> > We just got hit by the overriding. The result is that that the basic
> > types (int, double, ...) which had meaning before are suddenly
> > semantic free and non decomposable to lua scripting. Ugly.
>
> ?? Could you elaborate on this ?

Sure. Conversions between basic rtt types and Lua typesystem is
hardcoded, so that rtt "int", "uint", "float", "double" are always
converted to lua "number", a rtt "string" to a lua string and so
on. This is acceptable because it is only a small set of basic types,
and complex types (struct and sequences) normally decompose by
getMember into those.

Now if a type "ukkuagugga" is registered but which is really a "uint",
all types of ports, properties, operation arguments etc that were
uint's now report to be ukkuagugga's and lua has no clue what to do
with those. Sure, I can still call toString() but the meaning is lost.

Therefore I'd rather not depend on the type name, but rather on what
the type actually *is*.

> > So the overriding is causing trouble here, altough I do not claim that
> > overriding or not is the wrong or right thing to do. A flag to force
> > overriding or one to prevent overriding would have been helpful.
> >
> > Where should this be solved? One thing that comes to mind would be to
> > compare typeinfo pointers instead of names. As you register aliases, I
> > would assume the the following might work. E.g. to figure out if a
> > give DSB is actually a double could I do:
> >
> > 

> > TypeInfo* ti_unkown = unknown_dsb->getTypeInfo()
> > TypeInfo *ti_double = TypeInfoRepository::Instance()->type("double");
> > 
> > if(ti_unkown == t2_double)
> >       /* its a double */
> > else
> > 	/* its sth else */
> >

> >
> > This is not very nice.
> >
> > A better solution might be to extend what we had in mind for
> > identifying that a type is indexable. For example how about extending
> > TypeInfo with a method:
> >
> > TypeCat getTypeCategory()
> >
> > with enum TypeCat { array, struct, floatingpoint, integer, string, boolean
> > };
> >
> > That way I would know that a "my_float64_type", being typecat
> > 'floatingpoint' should be convertable to a double.
> >
> > Any better ideas?
>
> I'm open to suggestions myself.

Will solution 1 work?
Is solution 2 feasible?

Thank you in advance!
Markus

»

Updates on the 2.2.0 issues

Submitted by Sylvain Joyeux on Fri, 2011-01-14 10:36.

On 01/14/2011 11:28 AM, Peter Soetens wrote:
>> Bottom line: I could work with the proposed change if the RTT typekit
>> was split between fundamental types (integers, floats) and complex types
>> (string, vector, RTT-specific stuff). Which is a split that would make
>> sense IMO anyway.
>
> Hmm. So you were using this mechanism and you did not have any problems ? It
> could be that by accident the ROS typekit was broken in some way and that our
> solution only avoided loading these ROS types, hence excluding the wrong bug.
Yep. I had no problem whatsoever.

Bottom line:
* on Rock, we access only the new names
* we load the typekits at startup in our deployments. A side effect of
that is that no code (transport or otherwise) has time to "cache"
the TypeInfo structure.

So, in principle, there's no code path *in our case* that is seeing both
TypeInfo objects. I can understand that it is a pretty special case,
though ...

>> There's nothing conservative about it. The old behaviour was to have the
>> new TypeInfo, the new one to keep the old one. This changes the typeinfo
>> handling in fundamental ways.
>
> Not entirely. The old behavior kept 'both' typeinfo objects in the repository,
> but only stored one of them in the global type info variable. Maybe the better
> fix on this stable branch is to :
>
> - keep the old overriding behavior in TemplateTypeInfo
> - keep *only* the last type info object in the repository, and update the map
> such that a lookup for the first gives you the last.
> - delete the first type info object
>
> Better/Worse ?
That's better in my use case as it still allows overriding. It also
means, though, that one cannot cache TypeInfo object (you need to do a
query every time you need the TypeInfo, since it could be deleted).

»

Updates on the 2.2.0 issues

Submitted by peter on Mon, 2011-01-31 12:08.

On Friday 14 January 2011 11:34:30 Sylvain Joyeux wrote:
> On 01/14/2011 11:28 AM, Peter Soetens wrote:
> >> Bottom line: I could work with the proposed change if the RTT typekit
> >> was split between fundamental types (integers, floats) and complex types
> >> (string, vector, RTT-specific stuff). Which is a split that would make
> >> sense IMO anyway.
> >
> > Hmm. So you were using this mechanism and you did not have any problems ?
> > It could be that by accident the ROS typekit was broken in some way and
> > that our solution only avoided loading these ROS types, hence excluding
> > the wrong bug.
>
> Yep. I had no problem whatsoever.
>
> Bottom line:
> * on Rock, we access only the new names
> * we load the typekits at startup in our deployments. A side effect of
> that is that no code (transport or otherwise) has time to "cache"
> the TypeInfo structure.
>
> So, in principle, there's no code path *in our case* that is seeing both
> TypeInfo objects. I can understand that it is a pretty special case,
> though ...

For the record: the bug was in the ROS typekit and not because of the
replacing of the type info object.

>
> >> There's nothing conservative about it. The old behaviour was to have the
> >> new TypeInfo, the new one to keep the old one. This changes the typeinfo
> >> handling in fundamental ways.
> >
> > Not entirely. The old behavior kept 'both' typeinfo objects in the
> > repository, but only stored one of them in the global type info
> > variable. Maybe the better
> >
> > fix on this stable branch is to :
> > - keep the old overriding behavior in TemplateTypeInfo
> > - keep *only* the last type info object in the repository, and update
> > the map
> >
> > such that a lookup for the first gives you the last.
> >
> > - delete the first type info object
> >
> > Better/Worse ?
>
> That's better in my use case as it still allows overriding. It also
> means, though, that one cannot cache TypeInfo object (you need to do a
> query every time you need the TypeInfo, since it could be deleted).

We aren't caching, we always look it up in RTT code at least.

This is the resolution I propose for master (2.3):

Assume a type info + CORBA transport is present for std::string, with name
"string":

1. case register twice type name "string" which maps to std::string:
-> replace old type info object with new one
-> migrate transports from old to new
-> do no migrate 'constructors'

2. case register type name "/std/string" which maps to std::string, already
having typeinfo by name "string":
-> replace old type info object with new one
-> alias "string" to new type info object
-> migrate transports from old to new
-> inform all transports of new type "/std/string".
-> do no migrate 'constructors'

3. case register type name "string" which maps to "Foo::Bar"
-> complain that we don't support this

4. case where CORBA transport for /std/string is loaded:
-> complain that a CORBA transport is already present for this type.

The rationale is that transports are separate libraries that keep on living
accross type info updates. Registering a new type info object for same type
replaces the 'factory functions' of that type, while keeping the existing
transports for that type (which might already be in use!)

I also have a branch that splits up the RTT typekit into two typekits: one for
the 'primitive' C/C++ types and one for the RTT types (ConnPolicy, SendStatus,
etc), but that's not yet ready for merging (the transports need to be split
then too). A knowledgable user could then disable the compilation of the
primitive types and provide his own.

Peter

»

Updates on the 2.2.0 issues

Submitted by Sylvain Joyeux on Tue, 2011-02-01 11:36.

On 01/31/2011 01:07 PM, Peter Soetens wrote:
> On Friday 14 January 2011 11:34:30 Sylvain Joyeux wrote:
>> On 01/14/2011 11:28 AM, Peter Soetens wrote:
>>>> Bottom line: I could work with the proposed change if the RTT typekit
>>>> was split between fundamental types (integers, floats) and complex types
>>>> (string, vector, RTT-specific stuff). Which is a split that would make
>>>> sense IMO anyway.
>>>
>>> Hmm. So you were using this mechanism and you did not have any problems ?
>>> It could be that by accident the ROS typekit was broken in some way and
>>> that our solution only avoided loading these ROS types, hence excluding
>>> the wrong bug.
>>
>> Yep. I had no problem whatsoever.
>>
>> Bottom line:
>> * on Rock, we access only the new names
>> * we load the typekits at startup in our deployments. A side effect of
>> that is that no code (transport or otherwise) has time to "cache"
>> the TypeInfo structure.
>>
>> So, in principle, there's no code path *in our case* that is seeing both
>> TypeInfo objects. I can understand that it is a pretty special case,
>> though ...
>
> For the record: the bug was in the ROS typekit and not because of the
> replacing of the type info object.
>
>>
>>>> There's nothing conservative about it. The old behaviour was to have the
>>>> new TypeInfo, the new one to keep the old one. This changes the typeinfo
>>>> handling in fundamental ways.
>>>
>>> Not entirely. The old behavior kept 'both' typeinfo objects in the
>>> repository, but only stored one of them in the global type info
>>> variable. Maybe the better
>>>
>>> fix on this stable branch is to :
>>> - keep the old overriding behavior in TemplateTypeInfo
>>> - keep *only* the last type info object in the repository, and update
>>> the map
>>>
>>> such that a lookup for the first gives you the last.
>>>
>>> - delete the first type info object
>>>
>>> Better/Worse ?
>>
>> That's better in my use case as it still allows overriding. It also
>> means, though, that one cannot cache TypeInfo object (you need to do a
>> query every time you need the TypeInfo, since it could be deleted).
>
> We aren't caching, we always look it up in RTT code at least.
>
> This is the resolution I propose for master (2.3):
>
> Assume a type info + CORBA transport is present for std::string, with name
> "string":
>
> 1. case register twice type name "string" which maps to std::string:
> -> replace old type info object with new one
> -> migrate transports from old to new
> -> do no migrate 'constructors'
>
> 2. case register type name "/std/string" which maps to std::string, already
> having typeinfo by name "string":
> -> replace old type info object with new one
> -> alias "string" to new type info object
> -> migrate transports from old to new
> -> inform all transports of new type "/std/string".
> -> do no migrate 'constructors'
>
> 3. case register type name "string" which maps to "Foo::Bar"
> -> complain that we don't support this
I don't understand how this case is a different one.

> 4. case where CORBA transport for /std/string is loaded:
> -> complain that a CORBA transport is already present for this type.
And do what ? Replace with new one or replace with old one ?

4. is what is going to break rock, not the typeinfo. We don't care about
typeinfo at all. What is important is that *the* transport is *the*
transport generated by oroGen.

Note that it might work for std::string, as the CORBA-to-IDL mapping is
the same between the RTT and oroGen. But it is probably going to break
big time for vector<double>

> I also have a branch that splits up the RTT typekit into two typekits: one for
> the 'primitive' C/C++ types and one for the RTT types (ConnPolicy, SendStatus,
> etc), but that's not yet ready for merging (the transports need to be split
> then too). A knowledgable user could then disable the compilation of the
> primitive types and provide his own.
That's definitely the best fix.

»

Updates on the 2.2.0 issues

Submitted by peter on Tue, 2011-02-01 12:56.

On Tuesday 01 February 2011 12:34:07 Sylvain Joyeux wrote:
> On 01/31/2011 01:07 PM, Peter Soetens wrote:
> > On Friday 14 January 2011 11:34:30 Sylvain Joyeux wrote:
> >> On 01/14/2011 11:28 AM, Peter Soetens wrote:
> >>>> Bottom line: I could work with the proposed change if the RTT typekit
> >>>> was split between fundamental types (integers, floats) and complex
> >>>> types (string, vector, RTT-specific stuff). Which is a split that
> >>>> would make sense IMO anyway.
> >>>
> >>> Hmm. So you were using this mechanism and you did not have any problems
> >>> ? It could be that by accident the ROS typekit was broken in some way
> >>> and that our solution only avoided loading these ROS types, hence
> >>> excluding the wrong bug.
> >>
> >> Yep. I had no problem whatsoever.
> >>
> >> Bottom line:
> >> * on Rock, we access only the new names
> >> * we load the typekits at startup in our deployments. A side effect
> >> of
> >>
> >> that is that no code (transport or otherwise) has time to "cache"
> >> the TypeInfo structure.
> >>
> >> So, in principle, there's no code path *in our case* that is seeing both
> >> TypeInfo objects. I can understand that it is a pretty special case,
> >> though ...
> >
> > For the record: the bug was in the ROS typekit and not because of the
> > replacing of the type info object.
> >
> >>>> There's nothing conservative about it. The old behaviour was to have
> >>>> the new TypeInfo, the new one to keep the old one. This changes the
> >>>> typeinfo handling in fundamental ways.
> >>>
> >>> Not entirely. The old behavior kept 'both' typeinfo objects in the
> >>> repository, but only stored one of them in the global type info
> >>> variable. Maybe the better
> >>>
> >>> fix on this stable branch is to :
> >>> - keep the old overriding behavior in TemplateTypeInfo
> >>> - keep *only* the last type info object in the repository, and
> >>> update the map
> >>>
> >>> such that a lookup for the first gives you the last.
> >>>
> >>> - delete the first type info object
> >>>
> >>> Better/Worse ?
> >>
> >> That's better in my use case as it still allows overriding. It also
> >> means, though, that one cannot cache TypeInfo object (you need to do a
> >> query every time you need the TypeInfo, since it could be deleted).
> >
> > We aren't caching, we always look it up in RTT code at least.
> >
> > This is the resolution I propose for master (2.3):
> >
> > Assume a type info + CORBA transport is present for std::string, with
> > name "string":
> >
> > 1. case register twice type name "string" which maps to std::string:
> > -> replace old type info object with new one
> > -> migrate transports from old to new
> > -> do no migrate 'constructors'
> >
> > 2. case register type name "/std/string" which maps to std::string,
> > already having typeinfo by name "string":
> > -> replace old type info object with new one
> > -> alias "string" to new type info object
> > -> migrate transports from old to new
> > -> inform all transports of new type "/std/string".
> > -> do no migrate 'constructors'
> >
> > 3. case register type name "string" which maps to "Foo::Bar"
> > -> complain that we don't support this
>
> I don't understand how this case is a different one.

our types repos is an std::map<string,TypeInfo*>, so we can only have one type
info object for each type name. Case 3 tries to add 2 different type info
objects to the same type name. Case 2 does the opposite: map 2 type names to
one type info object.

>
> > 4. case where CORBA transport for /std/string is loaded:
> > -> complain that a CORBA transport is already present for this type.
>
> And do what ? Replace with new one or replace with old one ?

Keep old one. Corba transport classes cache the transport object pointer.

>
> 4. is what is going to break rock, not the typeinfo. We don't care about
> typeinfo at all. What is important is that *the* transport is *the*
> transport generated by oroGen.
>
> Note that it might work for std::string, as the CORBA-to-IDL mapping is
> the same between the RTT and oroGen. But it is probably going to break
> big time for vector<double>

The only solution I see in that case is to avoid the situation all together.
See below.

>
> > I also have a branch that splits up the RTT typekit into two typekits:
> > one for the 'primitive' C/C++ types and one for the RTT types
> > (ConnPolicy, SendStatus, etc), but that's not yet ready for merging (the
> > transports need to be split then too). A knowledgable user could then
> > disable the compilation of the primitive types and provide his own.
>
> That's definitely the best fix.

That also means thinking up package names for these typekits/transports.

I'm imagining something like this:

"rtt"
- Marshalling and scripting plugins
- The deployer will import this package automatically (required).
"rtt-types"
- The RTT native types&transport, ie types that are in the RTT:: namespace
- The deployer will import this package automatically(*)
"rtt-primitives"
- The C/C++ standard types&transport, ie int, float,... array, string
- The deployer will import this package automatically(*)

We could allow a commandline option to surpress the imports marked with (*),
ie --no-rtt-types. The user can then still import it explicitly by an
import("rtt-types") so there's all options available.

Peter

»

Updates on the 2.2.0 issues

Submitted by markus.klotzbuecher on Mon, 2011-01-31 13:52.

Hi Peter,

On Mon, Jan 31, 2011 at 01:07:22PM +0100, Peter Soetens wrote:
...
> This is the resolution I propose for master (2.3):
>
> Assume a type info + CORBA transport is present for std::string, with name
> "string":

The following would be done for each basic type: int, double, etc. ?

> 1. case register twice type name "string" which maps to std::string:
> -> replace old type info object with new one
> -> migrate transports from old to new
> -> do no migrate 'constructors'
>
> 2. case register type name "/std/string" which maps to std::string, already
> having typeinfo by name "string":
> -> replace old type info object with new one
> -> alias "string" to new type info object
> -> migrate transports from old to new
> -> inform all transports of new type "/std/string".
> -> do no migrate 'constructors'
>
> 3. case register type name "string" which maps to "Foo::Bar"
> -> complain that we don't support this

Hmm, do you really want to add such special cases? Wouldn't it be
cleaner to per default not override and add a import-override
operation which does?

> 4. case where CORBA transport for /std/string is loaded:
> -> complain that a CORBA transport is already present for this type.
>
> The rationale is that transports are separate libraries that keep on living
> accross type info updates. Registering a new type info object for same type
> replaces the 'factory functions' of that type, while keeping the existing
> transports for that type (which might already be in use!)

This makes sense.

> I also have a branch that splits up the RTT typekit into two typekits: one for
> the 'primitive' C/C++ types and one for the RTT types (ConnPolicy, SendStatus,
> etc), but that's not yet ready for merging (the transports need to be split
> then too). A knowledgable user could then disable the compilation of the
> primitive types and provide his own.

Markus

»