-
Notifications
You must be signed in to change notification settings - Fork 275
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: Use variable to catch fast NMT state transitions #436
base: melodic-devel
Are you sure you want to change the base?
fix: Use variable to catch fast NMT state transitions #436
Conversation
@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. |
@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. |
I will restart the job to see, if the error is sporadic. <?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
Value of: status.bounded<canopen::LayerStatus::Ok>()
 Actual: false
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? |
@ipa-mdl, friendly ping;) |
@ipa-mdl, can this be merged? |
Friendly ping statistics for @ipa-mdl: |
Solves #435, by storing the desired state in a variable (
wait_for_state_
). This waywait_for()
can cope with fast NMT state transitions.