Re: [Playerstage-developers] what happend to libplayertcp?!

The player TCP changes were probably part of the addition of the FileWatcher
capability, I will have a look at this as soon as I get a chance and see
what needs to be done to fix this. Of course calling request from a non
threaded driver is probably bad news anyway as request can block for a long
time, locking up the whole server (this is the reason that passthrough is
threaded).
Toby
2008/10/1 Paul Osmialowski <newchief@...>
> Hello devels,
>
> Recently I was experiencing serious problems with server->server
> communication. I was suspecting the problem must have been caused by
> recent changes in player core. So I started to dig for the reason.
> Finally, I must tell that I had to dig really deeply as I've found the
> reason inside of libplayertcp(). All the problems were caused by someone
> who removed virtual Update() method from TCPRemoteDriver class. Its
> implementation was calling Lock() and Unlock() methods from PlayerTCP
> class. These methods were moved to private section so I coulnd't use them
> from Update() method that I've reverted in TCPRemoteDriver. So I made them
> public back again. After these changes, all my problems has gone.
>
> Removing Update() method from TCPRemoteDriver caused it impossible to call
> Request() with threaded parameter set to false. It is rather rare case, as
> most of the Request calls are done while driver thread is running.
> Unfortunately, linuxjoystick driver calls Request() method from its
> Setup() method rihgt before driver thread is created - so the threaded
> parameter is false (and I guess it should be).
>
> I don't really know why Update() method was removed from TCPRemoteDriver
> and if it is an error or it should be like that which means, Request
> method should be rewritten so it handles not threaded case different way.
> Someone more responsible than my humble person should justify that.
>
> Also I've realised that setting rate parameter to 0 while running playerv
> solves all my problem with bad communication link between desktop and
> Roomba robot. I don't remember well, but something tells me that once it
> was like that by default and recent change of default value of this
> parameter to 5 has caused all my client->server problems.
>
> Cheers,
> Paul
>
> -------------------------------------------------------------------------
> This SF.Net email is sponsored by the Moblin Your Move Developer's
> challenge
> Build the coolest Linux based applications with Moblin SDK & win great
> prizes
> Grand prize is a trip for two to an Open Source event anywhere in the world
> http://moblin-contest.org/redirect.php?banner_id=100&url=/
> _______________________________________________
> Playerstage-developers mailing list
> Playerstage-developers@...
> https://lists.sourceforge.net/lists/listinfo/playerstage-developers
>
--
This email is intended for the addressee only and may contain privileged
and/or confidential information

Thread view

Hello devels,
Recently I was experiencing serious problems with server->server
communication. I was suspecting the problem must have been caused by
recent changes in player core. So I started to dig for the reason.
Finally, I must tell that I had to dig really deeply as I've found the
reason inside of libplayertcp(). All the problems were caused by someone
who removed virtual Update() method from TCPRemoteDriver class. Its
implementation was calling Lock() and Unlock() methods from PlayerTCP
class. These methods were moved to private section so I coulnd't use them
from Update() method that I've reverted in TCPRemoteDriver. So I made them
public back again. After these changes, all my problems has gone.
Removing Update() method from TCPRemoteDriver caused it impossible to call
Request() with threaded parameter set to false. It is rather rare case, as
most of the Request calls are done while driver thread is running.
Unfortunately, linuxjoystick driver calls Request() method from its
Setup() method rihgt before driver thread is created - so the threaded
parameter is false (and I guess it should be).
I don't really know why Update() method was removed from TCPRemoteDriver
and if it is an error or it should be like that which means, Request
method should be rewritten so it handles not threaded case different way.
Someone more responsible than my humble person should justify that.
Also I've realised that setting rate parameter to 0 while running playerv
solves all my problem with bad communication link between desktop and
Roomba robot. I don't remember well, but something tells me that once it
was like that by default and recent change of default value of this
parameter to 5 has caused all my client->server problems.
Cheers,
Paul

The player TCP changes were probably part of the addition of the FileWatcher
capability, I will have a look at this as soon as I get a chance and see
what needs to be done to fix this. Of course calling request from a non
threaded driver is probably bad news anyway as request can block for a long
time, locking up the whole server (this is the reason that passthrough is
threaded).
Toby
2008/10/1 Paul Osmialowski <newchief@...>
> Hello devels,
>
> Recently I was experiencing serious problems with server->server
> communication. I was suspecting the problem must have been caused by
> recent changes in player core. So I started to dig for the reason.
> Finally, I must tell that I had to dig really deeply as I've found the
> reason inside of libplayertcp(). All the problems were caused by someone
> who removed virtual Update() method from TCPRemoteDriver class. Its
> implementation was calling Lock() and Unlock() methods from PlayerTCP
> class. These methods were moved to private section so I coulnd't use them
> from Update() method that I've reverted in TCPRemoteDriver. So I made them
> public back again. After these changes, all my problems has gone.
>
> Removing Update() method from TCPRemoteDriver caused it impossible to call
> Request() with threaded parameter set to false. It is rather rare case, as
> most of the Request calls are done while driver thread is running.
> Unfortunately, linuxjoystick driver calls Request() method from its
> Setup() method rihgt before driver thread is created - so the threaded
> parameter is false (and I guess it should be).
>
> I don't really know why Update() method was removed from TCPRemoteDriver
> and if it is an error or it should be like that which means, Request
> method should be rewritten so it handles not threaded case different way.
> Someone more responsible than my humble person should justify that.
>
> Also I've realised that setting rate parameter to 0 while running playerv
> solves all my problem with bad communication link between desktop and
> Roomba robot. I don't remember well, but something tells me that once it
> was like that by default and recent change of default value of this
> parameter to 5 has caused all my client->server problems.
>
> Cheers,
> Paul
>
> -------------------------------------------------------------------------
> This SF.Net email is sponsored by the Moblin Your Move Developer's
> challenge
> Build the coolest Linux based applications with Moblin SDK & win great
> prizes
> Grand prize is a trip for two to an Open Source event anywhere in the world
> http://moblin-contest.org/redirect.php?banner_id=100&url=/
> _______________________________________________
> Playerstage-developers mailing list
> Playerstage-developers@...
> https://lists.sourceforge.net/lists/listinfo/playerstage-developers
>
--
This email is intended for the addressee only and may contain privileged
and/or confidential information

On Wed, 1 Oct 2008, Toby Collett wrote:
> The player TCP changes were probably part of the addition of the FileWatcher
> capability, I will have a look at this as soon as I get a chance and see
> what needs to be done to fix this. Of course calling request from a non
> threaded driver is probably bad news anyway as request can block for a long
> time, locking up the whole server (this is the reason that passthrough is
> threaded).
>
> Toby
>
Hi Toby,
I guess, this Request() call can be moved into Main() method and it should
be called before main loop is entered. It is possible that all other uses
of non-threaded Request() calls can be moved into threaded Main() method,
so we need to try to remove last parameter of Request() call and see how
many drivers needs to be redesigned (I guess most of the drivers call
Request() without explicitly giving the last parameters which default
value is true, so it may be not very painful operation).
I must also add that this issue touches both Player-2.1 and Player-2.2 SVN
branches!
Paul
> 2008/10/1 Paul Osmialowski <newchief@...>
>
>> Hello devels,
>>
>> Recently I was experiencing serious problems with server->server
>> communication. I was suspecting the problem must have been caused by
>> recent changes in player core. So I started to dig for the reason.
>> Finally, I must tell that I had to dig really deeply as I've found the
>> reason inside of libplayertcp(). All the problems were caused by someone
>> who removed virtual Update() method from TCPRemoteDriver class. Its
>> implementation was calling Lock() and Unlock() methods from PlayerTCP
>> class. These methods were moved to private section so I coulnd't use them
>> from Update() method that I've reverted in TCPRemoteDriver. So I made them
>> public back again. After these changes, all my problems has gone.
>>
>> Removing Update() method from TCPRemoteDriver caused it impossible to call
>> Request() with threaded parameter set to false. It is rather rare case, as
>> most of the Request calls are done while driver thread is running.
>> Unfortunately, linuxjoystick driver calls Request() method from its
>> Setup() method rihgt before driver thread is created - so the threaded
>> parameter is false (and I guess it should be).
>>
>> I don't really know why Update() method was removed from TCPRemoteDriver
>> and if it is an error or it should be like that which means, Request
>> method should be rewritten so it handles not threaded case different way.
>> Someone more responsible than my humble person should justify that.
>>
>> Also I've realised that setting rate parameter to 0 while running playerv
>> solves all my problem with bad communication link between desktop and
>> Roomba robot. I don't remember well, but something tells me that once it
>> was like that by default and recent change of default value of this
>> parameter to 5 has caused all my client->server problems.
>>
>> Cheers,
>> Paul
>>
>> -------------------------------------------------------------------------
>> This SF.Net email is sponsored by the Moblin Your Move Developer's
>> challenge
>> Build the coolest Linux based applications with Moblin SDK & win great
>> prizes
>> Grand prize is a trip for two to an Open Source event anywhere in the world
>> http://moblin-contest.org/redirect.php?banner_id=100&url=/
>> _______________________________________________
>> Playerstage-developers mailing list
>> Playerstage-developers@...
>> https://lists.sourceforge.net/lists/listinfo/playerstage-developers
>>
>
>
>
> --
> This email is intended for the addressee only and may contain privileged
> and/or confidential information
>

I started to check how much work need to be done to remove last parameter
from Request() call and I'm scared of what I've found:
0. Linuxjoystick - that's where it all started, Request() call can be
easily moved at the beginning of Main() method (with 'if' that checks if
position2d interface is in use).
1. Laserbar driver, Request() call with threded parameter set to false is
situated within ProcessMessage() method which is typically called within
driver thread. I was curious why it could be like that and guess what I've
found?! There's no StartThread() call within whole the driver! Strangely
enough, I can't find any place where Main() function is (explicitly)
called. Anyone uses this driver and can confirm it works well?!
2. Another problem: amcl_laser.cc, it must be a part of something bigger
as I
can't find Main() method, no StartThread() call and Request() is called
from Setup() and SetupMap() methods (with threaded parameter set to
false).
3. MapTransform (a class that inherits from Driver class) - Request()
called
from GetMap() method which itself is called from Setup() method,
nevertheless, Setup() method does not call StartThread() - does this
driver work?!
4. MrIcp (a class that inherits from Driver class) - Request() called from
SetupPositionDriver() and SetupLaser which itself is called directly from
class
constructor (sic!); it feels like these calls can be moved at the
beginning of Main() method. This time StartThread() is called from Setup()
method which is right.
5. WaveFront: Request() is called from SetupPosition(), GetMap() and
GetMapInfo() methods. SetupPosition() is called from Setup() and
it calls Subscribe(). The Request() call from SetupPosition() should be
moved at the beginning of Main() method. GetMap() and GetMapInfo() are
called from within driver thread (which is right) and from SetupMap()
method which itself is called from Setup() method (and calls Subscibe()
the same way as SetupPosition()) before the thread is created, so I guess
it should be rather called from the beginning of Main() method the same
way as in SetupPosition() case. StartThread() is called properly.
6. ND: Request() is called from SetOdom(), SetupLaser() and SetupSonar()
methods. These methods are called from Setup() method and they contain
Subscribe() calls, so it looks like Request() calls should be moved from
within those methods at the beginning of Main() method. StartThread() is
called properly.
7. VFH - the same as above, however this amuzed me:
if( ! synchronous_mode )
this->StartThread();
What is synchronous_mode?!
If it is legal, we need to reconsider if we really want to remove last
parameter of Request() call.... Who calls Main() method in this suspicious
"synchronous mode"?!
8. KartoLogger - Setup calls WriteGeometry() (which calls Request())
before StartThread(), it should be moved at the beginnng of Main() method.
9. Vec2Map calls Request() with theaded parameter explicitly set to true.
Easy to fix.
10. WriteLogDevice, as Vec2Map, easy to fix.
11. PassThrough, as WriteLogDevice and Vec2Map, easy to fix
That's all places that blocked compilation process after I'd removed last
parameter from Request() method. I'm not sure about that. First we need to
decide what to do with so called "synchronous mode" (no thread is created,
so Request() won't work at present).
Cheers,
Paul
On Tue, 30 Sep 2008, Paul Osmialowski wrote:
>
>
> On Wed, 1 Oct 2008, Toby Collett wrote:
>
>> The player TCP changes were probably part of the addition of the FileWatcher
>> capability, I will have a look at this as soon as I get a chance and see
>> what needs to be done to fix this. Of course calling request from a non
>> threaded driver is probably bad news anyway as request can block for a long
>> time, locking up the whole server (this is the reason that passthrough is
>> threaded).
>>
>> Toby
>>
> Hi Toby,
>
> I guess, this Request() call can be moved into Main() method and it should
> be called before main loop is entered. It is possible that all other uses
> of non-threaded Request() calls can be moved into threaded Main() method,
> so we need to try to remove last parameter of Request() call and see how
> many drivers needs to be redesigned (I guess most of the drivers call
> Request() without explicitly giving the last parameters which default
> value is true, so it may be not very painful operation).
>
> I must also add that this issue touches both Player-2.1 and Player-2.2 SVN
> branches!
>
> Paul
>
>> 2008/10/1 Paul Osmialowski <newchief@...>
>>
>>> Hello devels,
>>>
>>> Recently I was experiencing serious problems with server->server
>>> communication. I was suspecting the problem must have been caused by
>>> recent changes in player core. So I started to dig for the reason.
>>> Finally, I must tell that I had to dig really deeply as I've found the
>>> reason inside of libplayertcp(). All the problems were caused by someone
>>> who removed virtual Update() method from TCPRemoteDriver class. Its
>>> implementation was calling Lock() and Unlock() methods from PlayerTCP
>>> class. These methods were moved to private section so I coulnd't use them
>>> from Update() method that I've reverted in TCPRemoteDriver. So I made them
>>> public back again. After these changes, all my problems has gone.
>>>
>>> Removing Update() method from TCPRemoteDriver caused it impossible to call
>>> Request() with threaded parameter set to false. It is rather rare case, as
>>> most of the Request calls are done while driver thread is running.
>>> Unfortunately, linuxjoystick driver calls Request() method from its
>>> Setup() method rihgt before driver thread is created - so the threaded
>>> parameter is false (and I guess it should be).
>>>
>>> I don't really know why Update() method was removed from TCPRemoteDriver
>>> and if it is an error or it should be like that which means, Request
>>> method should be rewritten so it handles not threaded case different way.
>>> Someone more responsible than my humble person should justify that.
>>>
>>> Also I've realised that setting rate parameter to 0 while running playerv
>>> solves all my problem with bad communication link between desktop and
>>> Roomba robot. I don't remember well, but something tells me that once it
>>> was like that by default and recent change of default value of this
>>> parameter to 5 has caused all my client->server problems.
>>>
>>> Cheers,
>>> Paul
>>>
>>> -------------------------------------------------------------------------
>>> This SF.Net email is sponsored by the Moblin Your Move Developer's
>>> challenge
>>> Build the coolest Linux based applications with Moblin SDK & win great
>>> prizes
>>> Grand prize is a trip for two to an Open Source event anywhere in the world
>>> http://moblin-contest.org/redirect.php?banner_id=100&url=/
>>> _______________________________________________
>>> Playerstage-developers mailing list
>>> Playerstage-developers@...
>>> https://lists.sourceforge.net/lists/listinfo/playerstage-developers
>>>
>>
>>
>>
>> --
>> This email is intended for the addressee only and may contain privileged
>> and/or confidential information
>>
>
> -------------------------------------------------------------------------
> This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
> Build the coolest Linux based applications with Moblin SDK & win great prizes
> Grand prize is a trip for two to an Open Source event anywhere in the world
> http://moblin-contest.org/redirect.php?banner_id=100&url=/
> _______________________________________________
> Playerstage-developers mailing list
> Playerstage-developers@...
> https://lists.sourceforge.net/lists/listinfo/playerstage-developers
>

In messing with the sicklms200 driver, I've noticed that for some reason
changing from 38400 to 500000 isn't working (it could be my RS422 serial
to usb), but when this fails in Setup(), Shutdown() is never run, so the
device is never put back into 38400 mode (not that I'd be able to talk to
it anyway). But it seems strange to me that Shutdown() isn't run
automatically when Setup() returns 1. Any reason for this?
Patrick Beeson
http://www.cs.utexas.edu/users/pbeeson/
Men have become the tools of their tools.
Henry David Thoreau

The idea is that Setup should clean up its own resources when it fails, in
many drivers some resources are not initialized/allocated if setup fails
part way through and so calling shutdown could be dangerous (or require the
overhead of checking exactly what was set up). Basically the same principle
as not calling close an a file that failed to open or free on memory that
failed to allocate....
If calling shutdown is appropriate in this case you could do so explicitly
from Setup when it fails.
Toby
2008/10/2 Patrick Beeson <pbeeson@...>
> In messing with the sicklms200 driver, I've noticed that for some reason
> changing from 38400 to 500000 isn't working (it could be my RS422 serial
> to usb), but when this fails in Setup(), Shutdown() is never run, so the
> device is never put back into 38400 mode (not that I'd be able to talk to
> it anyway). But it seems strange to me that Shutdown() isn't run
> automatically when Setup() returns 1. Any reason for this?
>
> Patrick Beeson
> http://www.cs.utexas.edu/users/pbeeson/
>
> Men have become the tools of their tools.
> Henry David Thoreau
>
>
> -------------------------------------------------------------------------
> This SF.Net email is sponsored by the Moblin Your Move Developer's
> challenge
> Build the coolest Linux based applications with Moblin SDK & win great
> prizes
> Grand prize is a trip for two to an Open Source event anywhere in the world
> http://moblin-contest.org/redirect.php?banner_id=100&url=/
> _______________________________________________
> Playerstage-developers mailing list
> Playerstage-developers@...
> https://lists.sourceforge.net/lists/listinfo/playerstage-developers
>
--
This email is intended for the addressee only and may contain privileged
and/or confidential information

Thanks. I think its fine to have Setup() clean up itself if necessary.
Just curious.
Toby Collett wrote:
> The idea is that Setup should clean up its own resources when it fails,
> in many drivers some resources are not initialized/allocated if setup
> fails part way through and so calling shutdown could be dangerous (or
> require the overhead of checking exactly what was set up). Basically the
> same principle as not calling close an a file that failed to open or
> free on memory that failed to allocate....
>
> If calling shutdown is appropriate in this case you could do so
> explicitly from Setup when it fails.
>
> Toby
>
> 2008/10/2 Patrick Beeson <pbeeson@...
> <mailto:pbeeson@...>>
>
> In messing with the sicklms200 driver, I've noticed that for some reason
> changing from 38400 to 500000 isn't working (it could be my RS422 serial
> to usb), but when this fails in Setup(), Shutdown() is never run, so the
> device is never put back into 38400 mode (not that I'd be able to
> talk to
> it anyway). But it seems strange to me that Shutdown() isn't run
> automatically when Setup() returns 1. Any reason for this?
>
> Patrick Beeson
> http://www.cs.utexas.edu/users/pbeeson/
>
> Men have become the tools of their tools.
> Henry David Thoreau
>
>
> -------------------------------------------------------------------------
> This SF.Net email is sponsored by the Moblin Your Move Developer's
> challenge
> Build the coolest Linux based applications with Moblin SDK & win
> great prizes
> Grand prize is a trip for two to an Open Source event anywhere in
> the world
> http://moblin-contest.org/redirect.php?banner_id=100&url=/
> <http://moblin-contest.org/redirect.php?banner_id=100&url=/&gt;
> _______________________________________________
> Playerstage-developers mailing list
> Playerstage-developers@...
> <mailto:Playerstage-developers@...>
> https://lists.sourceforge.net/lists/listinfo/playerstage-developers
>
>
>
>
> --
> This email is intended for the addressee only and may contain privileged
> and/or confidential information
>
>
> ------------------------------------------------------------------------
>
> -------------------------------------------------------------------------
> This SF.Net email is sponsored by the Moblin Your Move Developer's challenge
> Build the coolest Linux based applications with Moblin SDK & win great prizes
> Grand prize is a trip for two to an Open Source event anywhere in the world
> http://moblin-contest.org/redirect.php?banner_id=100&url=/
>
>
> ------------------------------------------------------------------------
>
> _______________________________________________
> Playerstage-developers mailing list
> Playerstage-developers@...
> https://lists.sourceforge.net/lists/listinfo/playerstage-developers

On Sep 30, 2008, at 12:12 PM, Toby Collett wrote:
> The player TCP changes were probably part of the addition of the
> FileWatcher capability, I will have a look at this as soon as I get
> a chance and see what needs to be done to fix this. Of course
> calling request from a non threaded driver is probably bad news
> anyway as request can block for a long time, locking up the whole
> server (this is the reason that passthrough is threaded).
True enough. So should we add an update of the TCP server itself to
the loop inside Request()?
It seems like an ugly hack, but it's really important to support the
use of Request to and from non-threaded drivers. Not doing so makes
writing drivers much harder.
brian.
> 2008/10/1 Paul Osmialowski <newchief@...>
> Hello devels,
>
> Recently I was experiencing serious problems with server->server
> communication. I was suspecting the problem must have been caused by
> recent changes in player core. So I started to dig for the reason.
> Finally, I must tell that I had to dig really deeply as I've found the
> reason inside of libplayertcp(). All the problems were caused by
> someone
> who removed virtual Update() method from TCPRemoteDriver class. Its
> implementation was calling Lock() and Unlock() methods from PlayerTCP
> class. These methods were moved to private section so I coulnd't use
> them
> from Update() method that I've reverted in TCPRemoteDriver. So I
> made them
> public back again. After these changes, all my problems has gone.
>
> Removing Update() method from TCPRemoteDriver caused it impossible
> to call
> Request() with threaded parameter set to false. It is rather rare
> case, as
> most of the Request calls are done while driver thread is running.
> Unfortunately, linuxjoystick driver calls Request() method from its
> Setup() method rihgt before driver thread is created - so the threaded
> parameter is false (and I guess it should be).
>
> I don't really know why Update() method was removed from
> TCPRemoteDriver
> and if it is an error or it should be like that which means, Request
> method should be rewritten so it handles not threaded case different
> way.
> Someone more responsible than my humble person should justify that.
>
> Also I've realised that setting rate parameter to 0 while running
> playerv
> solves all my problem with bad communication link between desktop and
> Roomba robot. I don't remember well, but something tells me that
> once it
> was like that by default and recent change of default value of this
> parameter to 5 has caused all my client->server problems.
>
> Cheers,
> Paul
>
> -------------------------------------------------------------------------
> This SF.Net email is sponsored by the Moblin Your Move Developer's
> challenge
> Build the coolest Linux based applications with Moblin SDK & win
> great prizes
> Grand prize is a trip for two to an Open Source event anywhere in
> the world
> http://moblin-contest.org/redirect.php?banner_id=100&url=/
> _______________________________________________
> Playerstage-developers mailing list
> Playerstage-developers@...
> https://lists.sourceforge.net/lists/listinfo/playerstage-developers
>
>
>
> --
> This email is intended for the addressee only and may contain
> privileged and/or confidential information
> -------------------------------------------------------------------------
> This SF.Net email is sponsored by the Moblin Your Move Developer's
> challenge
> Build the coolest Linux based applications with Moblin SDK & win
> great prizes
> Grand prize is a trip for two to an Open Source event anywhere in
> the world
> http://moblin-contest.org/redirect.php?banner_id=100&url=/_______________________________________________
> Playerstage-developers mailing list
> Playerstage-developers@...
> https://lists.sourceforge.net/lists/listinfo/playerstage-developers

Paul Osmialowski wrote:
> I started to check how much work need to be done to remove last parameter
> from Request() call and I'm scared of what I've found:
>
> 0. Linuxjoystick - that's where it all started, Request() call can be
> easily moved at the beginning of Main() method (with 'if' that checks if
> position2d interface is in use).
>
> 1. Laserbar driver, Request() call with threded parameter set to false is
> situated within ProcessMessage() method which is typically called within
> driver thread. I was curious why it could be like that and guess what I've
> found?! There's no StartThread() call within whole the driver! Strangely
> enough, I can't find any place where Main() function is (explicitly)
> called. Anyone uses this driver and can confirm it works well?!
Drivers which don't call StartThread() are not threaded, they run in the
server thread and are usually event-driven (based on the arrival of
messages) with no heavy processing. You'll find a lot of the smaller and
simpler drivers are implemented in this way. Probably in this case the
Main() function is left over from it being threaded at one point. You
can find out for sure by doing a search of the subversion history for
the file. I do highly recommend learning how to use subversion commands
such as log, diff and annotate. They would have helped you find the
answer to many questions you've had recently about changes in code.
> 2. Another problem: amcl_laser.cc, it must be a part of something bigger
> as I
> can't find Main() method, no StartThread() call and Request() is called
> from Setup() and SetupMap() methods (with threaded parameter set to
> false).
amcl_laser.cc is, not surprisingly, part of the AMCL driver.
> 3. MapTransform (a class that inherits from Driver class) - Request()
> called
> from GetMap() method which itself is called from Setup() method,
> nevertheless, Setup() method does not call StartThread() - does this
> driver work?!
MapTransform is the base class for several actual drivers. Also, not
calling StartThread() is not necessarily an error, as mentioned above.
> 4. MrIcp (a class that inherits from Driver class) - Request() called from
> SetupPositionDriver() and SetupLaser which itself is called directly from
> class
> constructor (sic!); it feels like these calls can be moved at the
> beginning of Main() method. This time StartThread() is called from Setup()
> method which is right.
>
> 5. WaveFront: Request() is called from SetupPosition(), GetMap() and
> GetMapInfo() methods. SetupPosition() is called from Setup() and
> it calls Subscribe(). The Request() call from SetupPosition() should be
> moved at the beginning of Main() method. GetMap() and GetMapInfo() are
> called from within driver thread (which is right) and from SetupMap()
> method which itself is called from Setup() method (and calls Subscibe()
> the same way as SetupPosition()) before the thread is created, so I guess
> it should be rather called from the beginning of Main() method the same
> way as in SetupPosition() case. StartThread() is called properly.
>
> 6. ND: Request() is called from SetOdom(), SetupLaser() and SetupSonar()
> methods. These methods are called from Setup() method and they contain
> Subscribe() calls, so it looks like Request() calls should be moved from
> within those methods at the beginning of Main() method. StartThread() is
> called properly.
>
> 7. VFH - the same as above, however this amuzed me:
> if( ! synchronous_mode )
> this->StartThread();
> What is synchronous_mode?!
> If it is legal, we need to reconsider if we really want to remove last
> parameter of Request() call.... Who calls Main() method in this suspicious
> "synchronous mode"?!
>From a quick search of the docs for the VFH driver:
" - synchronous (int)
- default: 0
- If zero (the default), VFH runs in its own thread. If non-zero,
VFH runs in the main Player thread, which will make the server less
responsive, but prevent nasty asynchronous behaviour under high CPU
load. This is probably only useful when running demanding simulations."
> 8. KartoLogger - Setup calls WriteGeometry() (which calls Request())
> before StartThread(), it should be moved at the beginnng of Main() method.
>
> 9. Vec2Map calls Request() with theaded parameter explicitly set to true.
> Easy to fix.
>
> 10. WriteLogDevice, as Vec2Map, easy to fix.
>
> 11. PassThrough, as WriteLogDevice and Vec2Map, easy to fix
>
> That's all places that blocked compilation process after I'd removed last
> parameter from Request() method. I'm not sure about that. First we need to
> decide what to do with so called "synchronous mode" (no thread is created,
> so Request() won't work at present).
Theoretically, non-threaded drivers shouldn't do anything that could
block the server thread, like calling Request() - they should instead do
something like use PutMsg() and manually check for the arrival of the
ack/nack at a later stage.
Geoff

On Thu, 2 Oct 2008, gbiggs wrote:
>
>
> Theoretically, non-threaded drivers shouldn't do anything that could
> block the server thread, like calling Request() - they should instead do
> something like use PutMsg() and manually check for the arrival of the
> ack/nack at a later stage.
>
> Geoff
>
I see now we have two options:
1. Rewrite all non-threaded drivers to use PutMsg() instead of Request()
and for other affected drivers, move Request() calls from Setup() to Main()
2. Revert Update() method back into TCPRemoteDriver class so Request()
call will be possible from non-threaded environment back again.
It's easy to predict, second option will take less developers time to
implement.
Paul