Skip to content
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

[ROS][Test Infra] Support testing ROS2 service call device_info #3124

Merged

Conversation

kadiredd
Copy link
Contributor

@kadiredd kadiredd commented Jun 13, 2024

Implementation of JIRA ticket req: [LRS-1122]

@kadiredd kadiredd requested review from deep0294, SamerKhshiboun and Arun-Prasad-V and removed request for deep0294 June 13, 2024 08:33
@kadiredd kadiredd closed this Jun 13, 2024
@kadiredd kadiredd reopened this Jun 13, 2024
pytest.param(test_params_test_srv_d415, marks=pytest.mark.d415),
],indirect=True)
@pytest.mark.launch(fixture=launch_descr_with_parameters)
class TestCamera_ServiceCall(pytest_rs_utils.RsTestBaseClass):
Copy link
Contributor

Choose a reason for hiding this comment

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

this test only uses DeviceInfo service call. So, shall we rename this function and file names such that it is specific to DeviceInfo service.

Copy link
Contributor Author

@kadiredd kadiredd Jun 13, 2024

Choose a reason for hiding this comment

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

Yes, thought of it already - There are 8 service calls available. Plan is to have rest seven of them be added here in this file, so no need of changing the file name as of now. And, about changing the function name, yes/maybe -- to be addressed when implementing rest of the service calls. This is kind of placeholder test for validating service calls.

Copy link
Contributor Author

@kadiredd kadiredd Jun 14, 2024

Choose a reason for hiding this comment

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

got it, each test will carry distinct function name and distinct class name as well, right? Modified both class and function names.

@kadiredd kadiredd force-pushed the service_call branch 4 times, most recently from ba85807 to 5c298f7 Compare June 14, 2024 16:13
Copy link
Contributor

@Arun-Prasad-V Arun-Prasad-V left a comment

Choose a reason for hiding this comment

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

LGTM.

Copy link
Collaborator

@SamerKhshiboun SamerKhshiboun left a comment

Choose a reason for hiding this comment

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

LGTM, but don't we prefer to rename the file name to device info service ? or we want to add all services tests in the future in the same file ?

@kadiredd
Copy link
Contributor Author

LGTM, but don't we prefer to rename the file name to device info service ? or we want to add all services tests in the future in the same file ?

We want to add all other services in the same file, Yes.

@SamerKhshiboun SamerKhshiboun merged commit f8a054c into IntelRealSense:ros2-development Jun 18, 2024
8 checks passed
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.

None yet

3 participants