Behavior when failed to initialize

[guihomework]

Currently, if no sensor is connected, and one starts the ft_sensor launch file, one gets this

[ INFO] [1504252730.636147736]: ATISensor IP : 192.168.100.103
[ INFO] [1504252730.636233535]: ATISensor frame : /ati_link
Could not connect to 192.168.100.103:80
Initializing ft sensor
Start realtime streaming with 1 samples 
Error while receiving: Resource temporarily unavailable
Error of package size -1 but should be 36
Could not get response
Error during initialization, FT sensor NOT started
[ INFO] [1504252861.547558740]: ATISensor RDT Rate : 0

but the software still runs AND there is empty data produced on the topic.

So it is not easy for an external application managing startup to know something failed without explicitly parsing the stdout/stderr.

My question is : Is there a reason for the driver to run and publish empty data if no sensor was found ?

I would like to know your point of view before I make and PR changes.

[ahoarau]

Good question. I think there was a reason, but definitely not anymore. +1 on the PR to fix this and rtt_ati_sensor to catch this new exception thrown (?) and set itself to fatalerror.

[ahoarau]

I messed up the last merge, clean up coming

[ahoarau]

Should back to normal now :+1:

[ahoarau]

@guihomework did you try the latest build ?

[guihomework]

@ahoarau not yet. I will try do that this week

[guihomework]

Your latest code did not affect me except the renaming of the node from ft_sensor to ft_sensor_node (I already pushed a fix to master directly)

I will provide a PR immediately for the issue reported here, but only in the node, not at low-level. IMO each wrapper (node or orocos component) can implement its own test for failed initialization of the low-level library.

[ahoarau]

Don't you think it'll be better for the low level to just throw exceptions ? And higher level to catch them ?

[guihomework]

I imagined that the low-level driver can persist, and just be re-initialized with another IP calling init again (for instance for a node trying to discover sensors over a range of IP) I am not sure with a throw this will change much as this init function already has a return. The init can succeed or fail, then one should handle it in the node or the component.

[guihomework]

And if we throw at low-level, in which case do we throw ? There are currently a lot of init operations that simply execute whether the previous one failed or not. initialized_ is computed progressively and is return at the very end. If we throw in between, the high-level has to handle the different types of errors.

[ahoarau]

I'd say an exception (whatever type) for :

• https://github.com/kuka-isir/ati_sensor/blob/master/src/ft_sensor.cpp#L244
• https://github.com/kuka-isir/ati_sensor/blob/master/src/ft_sensor.cpp#L257

And just a warning (on std::cerr) for :

• https://github.com/kuka-isir/ati_sensor/blob/master/src/ft_sensor.cpp#L229
• https://github.com/kuka-isir/ati_sensor/blob/master/src/ft_sensor.cpp#L234
• https://github.com/kuka-isir/ati_sensor/blob/master/src/ft_sensor.cpp#L238

[guihomework]

If we throw, the init won't need a return anymore. This makes an API change, for those using this library. We could maintain it but a failure would never be seen on the return value any longer and code handling their failure through the init would need to change now too. I really did not want to introduce more complex mechanisms, just have the node not publish invalid data (so not run if init failed, which could be possible also without throw)

[ahoarau]

I'm fine with both solutions to be honest. Let's go for keeping the API that way. I'd still like the errors to be a bit more agressive, returning immediately false instead of trying to do the rest :

Could not connect to 192.168.100.103:80 --> return immediately false
Initializing ft sensor --> mute ?
Start realtime streaming with 1 samples --> mute ?
Error while receiving: Resource temporarily unavailable --> return false
Error of package size -1 but should be 36 --> return false
Could not get response --> mute ?
Error during initialization, FT sensor NOT started --> return false

Also (different PR), what about adding [FTSensor : 192.168.0.8] in front of every message ?

[guihomework]

I also think it should fail sooner. I am not sure about all the mute. I would keep at least the Start realtime streaming with 1 samples, one then knows the streaming has started

Since you already merged PR #15 should the more aggressive failure be a new PR ? do you want to handle it ?

[ahoarau]

I also think it should fail sooner. I am not sure about all the mute. I would keep at least the Start realtime streaming with 1 samples, one then knows the streaming has started

Agreed.

do you want to handle it ?

I trust you on this :)

[guihomework]

In order to cleanly return after the first error Could not connect to 192.168.10.100:80 the openSockets function should return correct information, but this line https://github.com/kuka-isir/ati_sensor/blob/master/src/ft_sensor.cpp#L302 does not affect the handle anymore and the handle might be valid (it is, otherwise we would see another error message), but cannot be opened. So the test on the handle is not sufficient. I will have to modify the behaviour of those functions too.

[guihomework]

All fixed now