Skip to content

Conversation

@j-rivero
Copy link
Contributor

Alternative to #84 after feedback received there.

I did not find problems on building or testing using this new approach. The PR looks cleaner.

@j-rivero
Copy link
Contributor Author

I've added 7480115 following up the conversation in #84 (comment). That triggered some failures in the system test https://github.com/ros2/system_tests/pull/501/files

    <testcase name="test_best_available_policies_subscription" status="run" result="completed" time="0.313" timestamp="2022-04-28T16:52:18" classname="test_quality_of_service.QosRclcppTestFixture__rmw_connextdds">                         
      <failure message="/home/jrivero/code/ros2/ws_qos_adaptative/src/ros2/system_tests/test_quality_of_service/test/test_best_available.cpp:76&#x0A;Expected equality of these values:&#x0A;  actual_qos.reliability()&#x0A;    Which is: 4-b
Expected equality of these values:                                               
  actual_qos.reliability()                                                       
    Which is: 4-byte object <01-00 00-00>                                        
  publisher_qos_profile.reliability()                                            
    Which is: 4-byte object <02-00 00-00>]]></failure>                           
      <failure message="/home/jrivero/code/ros2/ws_qos_adaptative/src/ros2/system_tests/test_quality_of_service/test/test_best_available.cpp:78&#x0A;Expected equality of these values:&#x0A;  actual_qos.deadline()&#x0A;    Which is: 16-byt
Expected equality of these values:                                               
  actual_qos.deadline()                                                          
    Which is: 16-byte object <F8-34 5A-26 37-7F 00-00 FF-FF FF-FF FF-FF FF-7F>   
  publisher_qos_profile.deadline()                                               
    Which is: 16-byte object <F8-34 5A-26 37-7F 00-00 80-84 1E-00 00-00 00-00>]]></failure>
      <failure message="/home/jrivero/code/ros2/ws_qos_adaptative/src/ros2/system_tests/test_quality_of_service/test/test_best_available.cpp:80&#x0A;Expected equality of these values:&#x0A;  actual_qos.liveliness_lease_duration()&#x0A;   
Expected equality of these values:                                               
  actual_qos.liveliness_lease_duration()                                         
    Which is: 16-byte object <F8-34 5A-26 37-7F 00-00 FF-FF FF-FF FF-FF FF-7F>   
  publisher_qos_profile.liveliness_lease_duration()                              
    Which is: 16-byte object <F8-34 5A-26 37-7F 00-00 C0-C6 2D-00 00-00 00-00>]]></failure>
    </testcase>                                                                  

Am I missing a change in this PR or the changes are wrong?

@j-rivero
Copy link
Contributor Author

One more final change to make test_quality_of_service to pass in d6f5efb.

Copy link
Member

@jacobperron jacobperron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@jacobperron jacobperron requested a review from asorbini April 29, 2022 21:59
@jacobperron jacobperron merged commit d267159 into master May 3, 2022
@delete-merged-branch delete-merged-branch bot deleted the jrivero/qos_best_available_common branch May 3, 2022 17:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants