search

robotology / icub-main

The iCub Main Software Repository
Other
110 stars 103 forks source link

Robot interface startup FAILS after update of EMS, MC4PLUS and 2FOC builds to the latest devel #597

Closed nunoguedelha closed 5 years ago

nunoguedelha commented 5 years ago

After updating the firmware EMS, MC4PLUS and 2FOC builds to the latest devel commit https://github.com/robotology/icub-firmware-build/commit/46d3f477b566be0b01d34409987e5d259abe36bb, the Robot interface startup fails. There is no ERROR message, only WARNINGS like the following:

3832    56563.465063    WARNING  from BOARD 10.0.1.25 (left_arm-eb25-j8_11) @ 1908s 727m 922u: CAN discovery is OK but FAKE (without any control on CAN w/ get-fw-version<> message) for 1 eobrd_mais boards with target can protocol ver 0.0 and application ver 0.0.0. Search time was 0 ms
3864    56563.688643    WARNING  from BOARD 10.0.1.26 (left_arm-eb26-j12_15) @ 1908s 957m 154u: CAN discovery is OK but FAKE (without any control on CAN w/ get-fw-version<> message) for 1 eobrd_mais boards with target can protocol ver 0.0 and application ver 0.0.0. Search time was 0 ms
3971    56564.450389    WARNING  from BOARD 10.0.1.28 (right_arm-eb28-j8_11) @ 1909s 707m 168u: CAN discovery is OK but FAKE (without any control on CAN w/ get-fw-version<> message) for 1 eobrd_mais boards with target can protocol ver 0.0 and application ver 0.0.0. Search time was 0 ms
4002    56564.673772    WARNING  from BOARD 10.0.1.29 (right_arm-eb29-j12_15) @ 1909s 928m 396u: CAN discovery is OK but FAKE (without any control on CAN w/ get-fw-version<> message) for 1 eobrd_mais boards with target can protocol ver 0.0 and application ver 0.0.0. Search time was 0 ms
4295    56567.250317    WARNING  from BOARD 10.0.1.24 (left_arm-eb24-j4_7) @ 1912s 512m 732u: CAN discovery is OK but FAKE (without any control on CAN w/ get-fw-version<> message) for 7 eobrd_mtb boards with target can protocol ver 0.0 and application ver 0.0.0. Search time was 0 ms
4317    56568.103766    WARNING  from BOARD 10.0.1.27 (right_arm-eb27-j4_7) @ 1913s 362m 870u: CAN discovery is OK but FAKE (without any control on CAN w/ get-fw-version<> message) for 7 eobrd_mtb boards with target can protocol ver 0.0 and application ver 0.0.0. Search time was 0 ms
4358    56568.959286    WARNING  from BOARD 10.0.1.11 (right_leg-eb11-skin) @ 1913s 984m 273u: CAN discovery is OK but FAKE (without any control on CAN w/ get-fw-version<> message) for 13 eobrd_mtb boards with target can protocol ver 0.0 and application ver 0.0.0. Search time was 0 ms
4390    56569.570182    WARNING  from BOARD 10.0.1.22 (face-eb22-j0_1) @ 1914s 832m 45u: CAN discovery is OK but FAKE (without any control on CAN w/ get-fw-version<> message) for 4 eobrd_mtb boards with target can protocol ver 0.0 and application ver 0.0.0. Search time was 0 ms

These are with respect to the MC4PLUS boards startup. For instance, for the MC4PLUS at IP 29 we get the following sequence of Warning/Info messages:

3992    56564.633390    WARNING embObjMC BOARD  right_arm-eb29-j12_15 Missing OTHER_CONTROL_PARAMETERS.DeadZone parameter. I'll use default value. (see documentation for more datails)
3994    56564.633415    INFO     from BOARD 10.0.1.29 (right_arm-eb29-j12_15), src LOCAL, adr 0, time 1909s 892m 618u: (code 0x00000034, par16 0x0001 par64 0x0000000000000023) -> SYS: the board has detected a restart of the remote transceiver because the rx ropframe sequence number is 1. In par64 there is the expected number. + .
4000    56564.663659    INFO    EthResource::askBoardVersion() found BOARD right_arm-eb29-j12_15 @ IP 10.0.1.29 of type mc4plus with FW = ver 3.21 built on 2019 Jul 24 17:34
4001    56564.673752    INFO     from BOARD 10.0.1.29 (right_arm-eb29-j12_15) @ 1909s 928m 234u: CAN discovery has started for 1 eobrd_mais boards on (can1map, can2map) = (0x4000, 0x0000) with target can protocol ver 0.0 and application ver 0.0.0.
4002    56564.673772    WARNING  from BOARD 10.0.1.29 (right_arm-eb29-j12_15) @ 1909s 928m 396u: CAN discovery is OK but FAKE (without any control on CAN w/ get-fw-version<> message) for 1 eobrd_mais boards with target can protocol ver 0.0 and application ver 0.0.0. Search time was 0 ms

Something goes wrong with the CAN discovery, and/or there is some missing configuration parameter.

The respective boards are detected properly through FirmwareUpdater.

nunoguedelha commented 5 years ago

@ale-git can you help me on this? Have the latest EMS, MC4PLUS and 2FOC builds been tested on any robot?

nunoguedelha commented 5 years ago

@marcoaccame since you're more familiar with this kind of issue, could you give us a hand?

nunoguedelha commented 5 years ago

I tried to start the interface from the terminal, and I get a Segmentation fault, so problem might be with the build or some plugin, and not the Firmware. I'll let you know.

nunoguedelha commented 5 years ago

CC @fjandrad

The startup is successful when using a startup script with everything activated except the wholebody dynamics estimator and the following FT and VFT sensors blocks:

    <!-- ANALOG SENSOR FT -->
    <xi:include href="wrappers/FT/left_arm-FT_wrapper.xml" />
    <xi:include href="wrappers/FT/right_arm-FT_wrapper.xml" />
    <xi:include href="wrappers/FT/right_leg-FT_wrapper.xml" />
    <xi:include href="wrappers/FT/left_leg-FT_wrapper.xml" />
    <xi:include href="wrappers/FT/right_foot-FT_wrapper.xml" />
    <xi:include href="wrappers/FT/left_foot-FT_wrapper.xml" />
    <xi:include href="wrappers/FT/right_foot-FT_wrapper_multipleSens.xml" />
    <xi:include href="wrappers/FT/right_leg-FT_wrapper_multipleSens.xml" />
    <xi:include href="wrappers/FT/left_leg-FT_wrapper_multipleSens.xml" />
    <xi:include href="wrappers/FT/left_foot-FT_wrapper_multipleSens.xml" />
    <xi:include href="hardware/FT/left_arm-eb1-j0_3-strain.xml" />
    <xi:include href="hardware/FT/right_arm-eb3-j0_3-strain.xml" />
    <xi:include href="hardware/FT/left_leg-eb6-j0_3-strain2.xml" />
    <xi:include href="hardware/FT/left_leg-eb7-j4_5-strain2.xml" />
    <xi:include href="hardware/FT/right_leg-eb8-j0_3-strain2.xml" />
    <xi:include href="hardware/FT/right_leg-eb9-j4_5-strain2.xml" />

    <!-- VIRTUAL ANALOG SENSORS (WRAPPER ONLY) -->
    <xi:include href="wrappers/VFT/left_arm-VFT_wrapper.xml" />
    <xi:include href="wrappers/VFT/right_leg-VFT_wrapper.xml" />
    <xi:include href="wrappers/VFT/left_leg-VFT_wrapper.xml" />
    <xi:include href="wrappers/VFT/right_arm-VFT_wrapper.xml" />
    <xi:include href="wrappers/VFT/torso-VFT_wrapper.xml" />

As soon as I add the ANALOG SENSOR FT block alone, I get the segmentation fault:

[INFO]The embObjStrain device using BOARD right_arm-eb3-j0_3  w/ IP 10.0.1.3 has the following service config: 
[INFO]- acquisitionrate = 10 
[INFO[]INFO] from BOARD 10.0.1.3 (right_arm-eb3-j0_3) @ 3757s 568m 714u: CAN discovery is OK for 1 eobrd_strain boards with target can protocol ver 1.0 and application ver 0.0.0. Search time was 1 ms- useCalibration = true 

[DEBUG] from BOARD 10.0.1.3 (right_arm-eb3-j0_3), src LOCAL, adr 0, time 3757s 568m 861u: (code 0x05000008, par16 0x010d par64 0x0000000101000000) -> CFG: EOtheSTRAIN can be correctly configured. can address in p16, prot and vers in p64 lower 32 bits. Byte 5 of p64: 0x01 is strain, 0x02 is strain2 + .
[INFO]- STRAIN of type strain named id_r_upper_arm_strain @ CAN2:13 with required protocol version = (1, 0) and required firmware version = (0, 0, 0) 
[WARNING] from BOARD 10.0.1.3 (right_arm-eb3-j0_3), src LOCAL, adr 0, time 3757s 569m 479u: (code 0x0000000b, par16 0x0217 par64 0x0077012a00950068) -> SYS: the RX phase of the control loop has last more than wanted. In par16: RX execution time [usec]. In par64: latest previous execution times of TX, RX, DO, TX [usec] + .
[INFO]created device <embObjStrain>. See C++ class yarp::dev::embObjStrain for documentation.
||| finding paths [plugins]
||| checking [/usr/local/src/robot/robotology-superbuild/robotology/robots-configuration/iCubGenova04/plugins] (pwd)
||| checking [/home/icub/.config/yarp/robots/iCubGenova04] (robot YARP_CONFIG_HOME)
||| checking [/home/icub/.local/share/yarp/robots/iCubGenova04] (robot YARP_DATA_HOME)
||| checking [/etc/yarp/robots/iCubGenova04] (robot YARP_CONFIG_DIRS)
||| checking [robots/iCubGenova04] (robot YARP_DATA_DIRS)
||| checking [/usr/local/src/robot/robotology-superbuild/build/install/share/yarp/robots/iCubGenova04] (robot YARP_DATA_DIRS)
||| found /usr/local/src/robot/robotology-superbuild/build/install/share/yarp/robots/iCubGenova04
||| checking [/usr/local/src/robot/robotology-superbuild/build/install/share/iCub/robots/iCubGenova04] (robot YARP_DATA_DIRS)
||| found /usr/local/src/robot/robotology-superbuild/build/install/share/iCub/robots/iCubGenova04
||| checking [/usr/local/src/robot/robotology-superbuild/build/install/share/ICUBcontrib/robots/iCubGenova04] (robot YARP_DATA_DIRS)
||| found /usr/local/src/robot/robotology-superbuild/build/install/share/ICUBcontrib/robots/iCubGenova04
||| checking [/usr/local/src/robot/robotology-superbuild/robotology/icub-tests/suits/robots/iCubGenova04] (robot YARP_DATA_DIRS)
||| checking [/usr/local/src/robot/robotology-superbuild/build/install/share/codyco/robots/iCubGenova04] (robot YARP_DATA_DIRS)
||| found /usr/local/src/robot/robotology-superbuild/build/install/share/codyco/robots/iCubGenova04
||| checking [/usr/local/src/robot/xsensmt-yarp-driver/build/share/yarp/robots/iCubGenova04] (robot YARP_DATA_DIRS)
||| checking [/usr/local/src/robot/element_floating-base-estimation/code/icubFbeV1/build/install/lib/yarp/robots/iCubGenova04] (robot YARP_DATA_DIRS)
||| checking [/usr/local/src/robot/element_floating-base-estimation/code/icubFbeV1/build/install/share/yarp/robots/iCubGenova04] (robot YARP_DATA_DIRS)
||| checking [config/path.d] (robot path.d YARP_DATA_DIRS)
||| checking [/usr/local/src/robot/robotology-superbuild/build/install/share/yarp/config/path.d] (robot path.d YARP_DATA_DIRS)
||| found /usr/local/src/robot/robotology-superbuild/build/install/share/yarp/config/path.d
||| checking [/usr/local/src/robot/robotology-superbuild/build/install/share/yarp/robots/iCubGenova04/plugins] (robot)
||| checking [/usr/local/src/robot/robotology-superbuild/build/install/share/iCub/robots/iCubGenova04/plugins] (robot)
||| checking [/usr/local/src/robot/robotology-superbuild/build/install/share/ICUBcontrib/robots/iCubGenova04/plugins] (robot)
||| checking [/usr/local/src/robot/robotology-superbuild/build/install/share/codyco/robots/iCubGenova04/plugins] (robot)
||| checking [/home/icub/.config/yarp/plugins] (YARP_CONFIG_HOME)
||| checking [/home/icub/.local/share/yarp/plugins] (YARP_DATA_HOME)
||| checking [/etc/yarp/plugins] (YARP_CONFIG_DIRS)
||| checking [plugins] (YARP_DATA_DIRS)
||| checking [/usr/local/src/robot/robotology-superbuild/build/install/share/yarp/plugins] (YARP_DATA_DIRS)
||| found /usr/local/src/robot/robotology-superbuild/build/install/share/yarp/plugins
||| checking [/usr/local/src/robot/robotology-superbuild/build/install/share/iCub/plugins] (YARP_DATA_DIRS)
||| found /usr/local/src/robot/robotology-superbuild/build/install/share/iCub/plugins
||| checking [/usr/local/src/robot/robotology-superbuild/build/install/share/ICUBcontrib/plugins] (YARP_DATA_DIRS)
||| found /usr/local/src/robot/robotology-superbuild/build/install/share/ICUBcontrib/plugins
||| checking [/usr/local/src/robot/robotology-superbuild/robotology/icub-tests/suits/plugins] (YARP_DATA_DIRS)
||| checking [/usr/local/src/robot/robotology-superbuild/build/install/share/codyco/plugins] (YARP_DATA_DIRS)
||| found /usr/local/src/robot/robotology-superbuild/build/install/share/codyco/plugins
||| checking [/usr/local/src/robot/xsensmt-yarp-driver/build/share/yarp/plugins] (YARP_DATA_DIRS)
||| found /usr/local/src/robot/xsensmt-yarp-driver/build/share/yarp/plugins
||| checking [/usr/local/src/robot/element_floating-base-estimation/code/icubFbeV1/build/install/lib/yarp/plugins] (YARP_DATA_DIRS)
||| checking [/usr/local/src/robot/element_floating-base-estimation/code/icubFbeV1/build/install/share/yarp/plugins] (YARP_DATA_DIRS)
Segmentation fault (core dumped)
nunoguedelha commented 5 years ago

(*) For instance adding:

    <xi:include href="wrappers/FT/right_leg-FT_wrapper.xml" />
    <xi:include href="hardware/FT/right_leg-eb8-j0_3-strain2.xml" />

@ale-git , @marcoaccame was there any impact on the plugin embObjFTsensor in the EMS changes?

nunoguedelha commented 5 years ago

...or is any update of the strain2 required?

nunoguedelha commented 5 years ago

Here is a print of the core dump after the segmentation fault:

[[INFO]- STRAIN of type strain named id_r_upper_arm_strain @ CAN2:13 with required protocol version = (1, 0) and required firmware version = (0, 0, 0) 
INFO] from BOARD 10.0.1.3 (right_arm-eb3-j0_3) @ 1562s 299m 648u: CAN discovery is OK for 1 eobrd_strain boards with target can protocol ver 1.0 and application ver 0.0.0. Search time was 1 ms
[DEBUG] from BOARD 10.0.1.3 (right_arm-eb3-j0_3), src LOCAL, adr 0, time 1562s 299m 793u: (code 0x05000008, par16 0x010d par64 0x0000000101000000) -> CFG: EOtheSTRAIN can be correctly configured. can address in p16, prot and vers in p64 lower 32 bits. Byte 5 of p64: 0x01 is strain, 0x02 is strain2 + .
[WARNING] from BOARD 10.0.1.3 (right_arm-eb3-j0_3), src LOCAL, adr 0, time 1562s 300m 421u: (code 0x0000000b, par16 0x020e par64 0x002301270095006b) -> SYS: the RX phase of the control loop has last more than wanted. In par16: RX execution time [usec]. In par64: latest previous execution times of TX, RX, DO, TX [usec] + .
[INFO]created device <embObjStrain>. See C++ class yarp::dev::embObjStrain for documentation.
[INFO]No ROS group found in config file ... skipping ROS initialization. 
||| finding paths [plugins]
yarp: Port /icub/right_leg/analog:o/rpc:i active at tcp://10.0.0.2:10230/
[INFO]AnalogServer : no ROS initialization required 
[INFO]created wrapper <analogServer>. See C++ class yarp::dev::AnalogWrapper for documentation.
||| finding paths [plugins]
==22539== Invalid read of size 8
==22539==    at 0x2F41CB2E: yarp::dev::embObjFTsensor::enableTemperatureTransmission(bool) (in /usr/local/src/robot/robotology-superbuild/build/install/lib/iCub/embObjFTsensor.so)
==22539==    by 0x2F41CD2A: yarp::dev::embObjFTsensor::cleanup() (in /usr/local/src/robot/robotology-superbuild/build/install/lib/iCub/embObjFTsensor.so)
==22539==    by 0x2F41CD48: yarp::dev::embObjFTsensor::close() (in /usr/local/src/robot/robotology-superbuild/build/install/lib/iCub/embObjFTsensor.so)
==22539==    by 0x2F41CDA6: yarp::dev::embObjFTsensor::~embObjFTsensor() (in /usr/local/src/robot/robotology-superbuild/build/install/lib/iCub/embObjFTsensor.so)
==22539==    by 0x2F41CDE8: yarp::dev::embObjFTsensor::~embObjFTsensor() (in /usr/local/src/robot/robotology-superbuild/build/install/lib/iCub/embObjFTsensor.so)
==22539==    by 0x4E9E258: DriversHelper::load(char const*) (in /usr/local/src/robot/robotology-superbuild/build/install/lib/libYARP_dev.so.3.2.100)
==22539==    by 0x4E9F9CC: yarp::dev::Drivers::find(char const*) (in /usr/local/src/robot/robotology-superbuild/build/install/lib/libYARP_dev.so.3.2.100)
==22539==    by 0x4ECC370: yarp::dev::PolyDriver::coreOpen(yarp::os::Searchable&) (in /usr/local/src/robot/robotology-superbuild/build/install/lib/libYARP_dev.so.3.2.100)
==22539==    by 0x4ECD1D1: yarp::dev::PolyDriver::open(yarp::os::Searchable&) (in /usr/local/src/robot/robotology-superbuild/build/install/lib/libYARP_dev.so.3.2.100)
==22539==    by 0x115774: RobotInterface::Device::open() (in /usr/local/src/robot/robotology-superbuild/build/install/bin/yarprobotinterface)
==22539==    by 0x119FAC: RobotInterface::Robot::Private::openDevices() (in /usr/local/src/robot/robotology-superbuild/build/install/bin/yarprobotinterface)
==22539==    by 0x11D7D8: RobotInterface::Robot::enterPhase(RobotInterface::ActionPhase) (in /usr/local/src/robot/robotology-superbuild/build/install/bin/yarprobotinterface)
==22539==  Address 0x0 is not stack'd, malloc'd or (recently) free'd
==22539== 
==22539== 
==22539== Process terminating with default action of signal 11 (SIGSEGV)
==22539==  Access not within mapped region at address 0x0
==22539==    at 0x2F41CB2E: yarp::dev::embObjFTsensor::enableTemperatureTransmission(bool) (in /usr/local/src/robot/robotology-superbuild/build/install/lib/iCub/embObjFTsensor.so)
==22539==    by 0x2F41CD2A: yarp::dev::embObjFTsensor::cleanup() (in /usr/local/src/robot/robotology-superbuild/build/install/lib/iCub/embObjFTsensor.so)
==22539==    by 0x2F41CD48: yarp::dev::embObjFTsensor::close() (in /usr/local/src/robot/robotology-superbuild/build/install/lib/iCub/embObjFTsensor.so)
==22539==    by 0x2F41CDA6: yarp::dev::embObjFTsensor::~embObjFTsensor() (in /usr/local/src/robot/robotology-superbuild/build/install/lib/iCub/embObjFTsensor.so)
==22539==    by 0x2F41CDE8: yarp::dev::embObjFTsensor::~embObjFTsensor() (in /usr/local/src/robot/robotology-superbuild/build/install/lib/iCub/embObjFTsensor.so)
==22539==    by 0x4E9E258: DriversHelper::load(char const*) (in /usr/local/src/robot/robotology-superbuild/build/install/lib/libYARP_dev.so.3.2.100)
==22539==    by 0x4E9F9CC: yarp::dev::Drivers::find(char const*) (in /usr/local/src/robot/robotology-superbuild/build/install/lib/libYARP_dev.so.3.2.100)
==22539==    by 0x4ECC370: yarp::dev::PolyDriver::coreOpen(yarp::os::Searchable&) (in /usr/local/src/robot/robotology-superbuild/build/install/lib/libYARP_dev.so.3.2.100)
==22539==    by 0x4ECD1D1: yarp::dev::PolyDriver::open(yarp::os::Searchable&) (in /usr/local/src/robot/robotology-superbuild/build/install/lib/libYARP_dev.so.3.2.100)
==22539==    by 0x115774: RobotInterface::Device::open() (in /usr/local/src/robot/robotology-superbuild/build/install/bin/yarprobotinterface)
==22539==    by 0x119FAC: RobotInterface::Robot::Private::openDevices() (in /usr/local/src/robot/robotology-superbuild/build/install/bin/yarprobotinterface)
==22539==    by 0x11D7D8: RobotInterface::Robot::enterPhase(RobotInterface::ActionPhase) (in /usr/local/src/robot/robotology-superbuild/build/install/bin/yarprobotinterface)
==22539==  If you believe this happened as a result of a stack
==22539==  overflow in your program's main thread (unlikely but
==22539==  possible), you can try to increase the size of the
==22539==  main thread stack using the --main-stacksize= flag.
==22539==  The main thread stack size used in this run was 8388608.
==22539== 
==22539== HEAP SUMMARY:
==22539==     in use at exit: 8,108,183 bytes in 96,505 blocks
==22539==   total heap usage: 2,085,805 allocs, 1,989,300 frees, 213,105,590 bytes allocated
==22539== 
==22539== LEAK SUMMARY:
==22539==    definitely lost: 838 bytes in 18 blocks
==22539==    indirectly lost: 47,141 bytes in 336 blocks
==22539==      possibly lost: 21,632 bytes in 81 blocks
==22539==    still reachable: 8,038,572 bytes in 96,070 blocks
==22539==                       of which reachable via heuristic:
==22539==                         newarray           : 38,568 bytes in 92 blocks
==22539==                         multipleinheritance: 3,728 bytes in 6 blocks
==22539==         suppressed: 0 bytes in 0 blocks
==22539== Rerun with --leak-check=full to see details of leaked memory
==22539== 
==22539== For counts of detected and suppressed errors, rerun with: -v
==22539== Use --track-origins=yes to see where uninitialised values come from
==22539== ERROR SUMMARY: 111 errors from 3 contexts (suppressed: 0 from 0)
Segmentation fault (core dumped)
nunoguedelha commented 5 years ago

As a workaround, I replaced all hardware/FT/<part>-ebx-jx_x-strain2.xml config files by hardware/FT/<part>-ebx-jx_x-strain.xml, thus selecting the old device embObjStrain instead of embObjFTsensor. The later is causing the observed issue.

nunoguedelha commented 5 years ago

This issue prevents to use the temperature measurements from the FTs.

traversaro commented 5 years ago

I think @fjandrad had a similar problem.

nunoguedelha commented 5 years ago

Indeed he told me so, and suggested the workaround.

valegagge commented 5 years ago

Hi @nunoguedelha , I fixed the bug you reported in the embObjFT device. Please see this commit .

So please update the icub-main repository and let me know if you notice the issue again.

Some more details: the crash was due to an access to an empty pointer, because yarp::dev::DriversHelper::load() function creates and than destroys the device without calls the open function, where the pointer will be initialized.

I didn't know this behavior of device loading, so I supposed that the open is called always before the destroy. Now I learned somthing new :)

Thanks for the detailed description of the bug. It helped me to understand and fix it quickly

nunoguedelha commented 5 years ago

Thanks a lot for the quick response @valegagge !

If I understood well, the call to enableTemperatureTransmission(false); in: https://github.com/robotology/icub-main/blob/bb055cddc4868fc54f67b2e1f7beca2b3f384ea0/src/libraries/icubmod/embObjFTsensor/embObjFTsensor.cpp#L207 should disable the temperature transmission.

I think we are missing an error use case in (https://github.com/robotology/icub-main/blob/bb055cddc4868fc54f67b2e1f7beca2b3f384ea0/src/libraries/icubmod/embObjFTsensor/embObjFTsensor.cpp#L68-L88): if some code creates the device and tries to enable the temperature trans before opening the device.

A more robust behavior would be:

It's always better (saves debugging time) having an error than a coredump, even for a use case which is not supposed to happen.

nunoguedelha commented 5 years ago

BTW I suggest we move this issue to icub-main.

traversaro commented 5 years ago

@nunoguedelha done

valegagge commented 5 years ago

Hi @nunoguedelha , the enableTemperatureTransmission function is private, so none can calls it. It is used inside to enable and disable the transmission of temperature values during opening or closing device.

The idea behind the return value is: return true if the command has been sent, false otherwise. If the resource doesn't exist then it is not possible sent the command. This happen only: 1) if the resource has been close twice 2) if the device has been created, but never opened.

In both case, at the end we are sure that the transmission is disable. Anyway, I think is not simple find the correct trade off between provide information to the user and to make the log readable. Therefore if you prefer to add a warning message, feel free to change the file. :)

About the second case you mentioned, it is not possible because the enableTemperatureTransmission is called after the creation of resource.

Instead, I see that the return value is not check in open function, so I verify it here

nunoguedelha commented 5 years ago

Hi @valegagge , it's clear thanks.

I was thinking also about a safeguard mechanism for the case where enableTemperatureTransmission is called from inside the device embObjFTsensor before the resource is allocated, by mistake (after some code refactoring for instance). But this is just my personal point of view regarding protection against human coding errors.

nunoguedelha commented 5 years ago

I will test tomorrow the fix.

nunoguedelha commented 5 years ago

@fjandrad tested the fix and it was working fine. Both fixes by @valegagge are commits bb055cd and 55154ef.

Current branch devel of the repo is already ahead of these commits.

Closing the issue.