ros-industrial / ros_canopen

CANopen driver framework for ROS (http://wiki.ros.org/ros_canopen)
GNU Lesser General Public License v3.0
328 stars 267 forks source link

fix: Use variable to catch fast NMT state transitions #436

Open PaulVerhoeckx opened 3 years ago

PaulVerhoeckx commented 3 years ago

Solves #435, by storing the desired state in a variable (wait_for_state_). This way wait_for() can cope with fast NMT state transitions.

PaulVerhoeckx commented 3 years ago

@ipa-mdl, does the failing test require effort from my side, or could it be a flaky test? I can't reproduce the failing test locally.

mathias-luedtke commented 3 years ago

@PaulRuvu: I am not sure yet.. your patch seems to change the timing in some cases, but it might be flaw in the test case as well.

mathias-luedtke commented 3 years ago

I will restart the job to see, if the error is sporadic. Old results:

<?xml version="1.0" encoding="UTF-8"?>
<testsuites tests="1" failures="1" disabled="0" errors="0" timestamp="2021-06-09T17:08:46" time="2.033" name="AllTests">
  <testsuite name="TestNode" tests="1" failures="1" disabled="0" errors="0" time="2.033">
    <testcase name="testInitandShutdown" status="run" time="2.033" classname="TestNode">
      <failure message="/root/target_ws/src/ros_canopen/canopen_master/test/test_node.cpp:49&#x0A;Value of: status.bounded&lt;canopen::LayerStatus::Ok&gt;()&#x0A;  Actual: false&#x0A;Expected: true" type=""><![CDATA[/root/target_ws/src/ros_canopen/canopen_master/test/test_node.cpp:49
Value of: status.bounded<canopen::LayerStatus::Ok>()
  Actual: false
Expected: true]]></failure>
    </testcase>
  </testsuite>
</testsuites>
PaulVerhoeckx commented 2 years ago

I will restart the job to see, if the error is sporadic. Old results:

<?xml version="1.0" encoding="UTF-8"?>
<testsuites tests="1" failures="1" disabled="0" errors="0" timestamp="2021-06-09T17:08:46" time="2.033" name="AllTests">
  <testsuite name="TestNode" tests="1" failures="1" disabled="0" errors="0" time="2.033">
    <testcase name="testInitandShutdown" status="run" time="2.033" classname="TestNode">
      <failure message="/root/target_ws/src/ros_canopen/canopen_master/test/test_node.cpp:49&#x0A;Value of: status.bounded&lt;canopen::LayerStatus::Ok&gt;()&#x0A;  Actual: false&#x0A;Expected: true" type=""><![CDATA[/root/target_ws/src/ros_canopen/canopen_master/test/test_node.cpp:49
Value of: status.bounded<canopen::LayerStatus::Ok>()
  Actual: false
Expected: true]]></failure>
    </testcase>
  </testsuite>
</testsuites>

@ipa-mdl, can we conclude it was sporadic?

PaulVerhoeckx commented 2 years ago

@ipa-mdl, friendly ping;)

PaulVerhoeckx commented 2 years ago

@ipa-mdl, can this be merged?

PaulVerhoeckx commented 2 years ago

Friendly ping statistics for @ipa-mdl:    Packets: Sent = 3, Received = 0, Lost = 3 (100% loss)