-
Notifications
You must be signed in to change notification settings - Fork 79
Support get_type_description service via parameter #1162
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
Changes from all commits
bb3794d
3bd5939
5120e28
f678f10
aa2a857
341f936
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -19,6 +19,7 @@ const assertUtils = require('./utils.js'); | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const DistroUtils = require('../lib/distro.js'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const rclnodejs = require('../index.js'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const TypeDescriptionService = require('../lib/type_description_service.js'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const { exec } = require('child_process'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| describe('type description service test suite', function () { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| this.timeout(60 * 1000); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -35,14 +36,13 @@ describe('type description service test suite', function () { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const nodeName = 'test_type_description_service'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| node = rclnodejs.createNode(nodeName); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| rclnodejs.spin(node); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| await assertUtils.createDelay(1000); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| afterEach(function () { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| rclnodejs.shutdown(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| it('Test type description service', function (done) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| it('Test type description service', async function () { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Create a publisher | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const topic = 'test_get_type_description_publisher'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const topicType = 'std_msgs/msg/String'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -63,14 +63,68 @@ describe('type description service test suite', function () { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const GetTypeDescription = | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 'type_description_interfaces/srv/GetTypeDescription'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const client = node.createClient(GetTypeDescription, serviceName); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| client.sendRequest(request, (response) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert.strictEqual(response.successful, true); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert.strictEqual( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| response.type_description.type_description.type_name, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| topicType | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert.notStrictEqual(response.type_sources.length, 0); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| done(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return client.waitForService(60 * 1000).then((result) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (!result) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| throw new Error('Service not available'); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return new Promise((resolve) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| client.sendRequest(request, (response) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert.strictEqual(response.successful, true); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert.strictEqual( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| response.type_description.type_description.type_name, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| topicType | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| assert.notStrictEqual(response.type_sources.length, 0); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| resolve(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| }); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| it('Test type description service configured by parameter', function (done) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| exec( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 'ros2 param list /test_type_description_service', | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| (error, stdout, stderr) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (error || stderr) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| done( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| new Error( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| 'Test type description service configured by parameter failed.' | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+88
to
+94
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if (error || stderr) { | |
| done( | |
| new Error( | |
| 'Test type description service configured by parameter failed.' | |
| ) | |
| ); | |
| } | |
| if (error) { | |
| done( | |
| new Error( | |
| `Test type description service configured by parameter failed with error: ${error.message}` | |
| ) | |
| ); | |
| return; | |
| } | |
| if (stderr) { | |
| console.warn(`Warning: stderr output detected: ${stderr}`); | |
| } |
Copilot
AI
Jun 11, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The test case may hang if 'start_type_description_service' is not found in stdout because there is no else branch to call done() with an error. Consider adding an else branch that terminates the test with an appropriate error message.
| done(); | |
| done(); | |
| } else { | |
| done( | |
| new Error( | |
| "'start_type_description_service' not found in stdout." | |
| ) | |
| ); |
Copilot
AI
Jun 11, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shelling out to ros2 in tests can be brittle; consider using the rclnodejs Node API to list or get parameters directly in code for a more reliable test.
| exec( | |
| 'ros2 param list /test_type_description_service', | |
| (error, stdout, stderr) => { | |
| if (error || stderr) { | |
| done( | |
| new Error( | |
| 'Test type description service configured by parameter failed.' | |
| ) | |
| ); | |
| } | |
| if (stdout.includes('start_type_description_service')) { | |
| done(); | |
| } else { | |
| done( | |
| new Error("'start_type_description_service' not found in stdout.") | |
| ); | |
| } | |
| } | |
| ); | |
| const parameterName = 'start_type_description_service'; | |
| const parameters = node.getParameters([parameterName]); | |
| if (parameters[parameterName] !== undefined) { | |
| done(); | |
| } else { | |
| done( | |
| new Error(`'${parameterName}' not found in node parameters.`) | |
| ); | |
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Consider extracting the parameter name 'start_type_description_service' into a top-level constant to avoid duplication and reduce risk of typos.