robotology / wearables

Code moved to https://github.com/robotology/human-dynamics-estimation
https://github.com/robotology/human-dynamics-estimation
BSD 3-Clause "New" or "Revised" License
19 stars 11 forks source link

improve errors status handling #43

Closed lrapetti closed 5 years ago

lrapetti commented 5 years ago

Currently, the IWear create from IAnalogSensor, works only as long as the IAnalogSensor interface is in OK status (yarp::dev::IAnalogSensor::AS_OK). Any other status stop the interface since readData() returns false and then the getXSensor returns false.

This behaviour is not expected when the status of the analog sensor is for example yarp::dev::IAnalogSensor::AS_TIMEOUT or yarp::dev::IAnalogSensor::AS_OVF, in fact in tha case the analog sensor interface status can go back to OK, but the wearable interface is never restored. Moreover, the status of the wearable sensor most likely is set only in case of AS_OK since it is the only case in which setStatus() is reached.

In order to fix this behaviour, a local fix that was used is to use a while loop instead of the if in getXSensor:

while (!handler.readData()) {
            sleep(20)
        }

However, as discussed with @diegoferigo, a proper fix would be to stream a dumb data (e.g. zero) even in case the status is not OK, and set the status of the interface. Then the consumer device should take care of checking the status of the data and eventually handle the error.

lrapetti commented 5 years ago

Actually, I see the problem may be solved also at the wrapper level. In fact, the wrapper stops (see e.g. https://github.com/robotology/wearables/blob/master/wrappers/IWear/src/IWearWrapper.cpp#L222) when the IAnalog sensor is not ok (see here or when getForceTorque6D return false), and probably we could avoid it to stop a part maybe just for the case of status:ERROR.

lrapetti commented 5 years ago

Moreover, also at the IWearWrapper level we should not stop if the status is not OK, we may stop just in case the status is ERROR (see https://github.com/robotology/wearables/blob/master/wrappers/IWear/src/IWearWrapper.cpp#L144)

lrapetti commented 5 years ago

@diegoferigo do you agree also about this last two comments?

lrapetti commented 5 years ago

Also in the IWearRemapper only ERROR and OK status are considered (see https://github.com/robotology/wearables/blob/master/devices/IWearRemapper/src/IWearRemapper.cpp#L758). So, in general, the error handling seems to need some improvements (it was still a TODO).

lrapetti commented 5 years ago

For the moment I have opened a PR where the details for improving IAnalogSensor to IWear are discussed in https://github.com/robotology/wearables/pull/44. Probably once this is done, we can discuss how to improve IWearRemapper and IWearWrapper.

yeshasvitirupachuri commented 5 years ago

@diegoferigo do you agree also about this last two comments?

@lrapetti I believe according to the design, each wearable device should handle the individual sensor cases like timeout correctly, like by waiting for a set duration to see if timeout status can be changed to ok.

diegoferigo commented 5 years ago

Actually, I see the problem may be solved also at the wrapper level. In fact, the wrapper stops (see e.g. https://github.com/robotology/wearables/blob/master/wrappers/IWear/src/IWearWrapper.cpp#L222) when the IAnalog sensor is not ok (see here or when getForceTorque6D return false), and probably we could avoid it to stop a part maybe just for the case of status:ERROR.

Error handling and timestamps still need to be carefully handled. In my opinion the consumers should always check the status of the sensors before using their data.

@lrapetti For instance, here what we can do is return false only if the status of the sensor is not ok. In this way, the consumer can read the sensor status containing the error only if reading the force failed, and act accordingly.

@diegoferigo do you agree also about this last two comments?

I agree.

I believe according to the design, each wearable device should handle the individual sensor cases like timeout correctly, like by waiting for a set duration to see if timeout status can be changed to ok.

In this case this depends on the type of the source. The producer typically just need to gather data and associate it with the right sensor status. If there are errors, it should not fail. Rather, it should return e.g. dummy data (all zeros) and set the correct sensor status. Then, it's the consumer that needs to handle failures properly.

lrapetti commented 5 years ago

The improvements have been addressed in https://github.com/robotology/wearables/pull/45 and https://github.com/robotology/wearables/pull/44.

Further testing will be performed in the next days and in case of any problem we may reopen the issue.