Skip to content

Commit

Permalink
Fix publishing a fixed array in a topic (#984)
Browse files Browse the repository at this point in the history 
Currently, when publishing a topic that contains a fixed array, the sub
cannot receive the topic as expected values.

This patch fixes this issue for publishing a fixed array in a topic.
Meanwhile, a unit test is added to ensure it works for different types
of array.

Fix: #979
  • Loading branch information
@minggangw
minggangw authored Aug 6, 2024
1 parent bfed3ae commit f9ea71b
Show file tree
Hide file tree
Showing 4 changed files with 170 additions and 13 deletions.
2 changes: 1 addition & 1 deletion rosidl_gen/generator.json
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "rosidl-generator",
"version": "0.3.8",
"version": "0.3.9",
"description": "Generate JavaScript object from ROS IDL(.msg) files",
"main": "index.js",
"authors": [
Expand Down
5 changes: 4 additions & 1 deletion rosidl_gen/message_translator.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -145,10 +145,13 @@ function toPlainObject(message, enableTypedArray = true) {
obj[name] = Array.from(message[name]);
} else {
// Direct assignment
// Note: TypedArray also falls into this branch if |enableTypedArray| is true
// Note: TypedArray also falls into this branch if |enableTypedArray| is true
// TODO(Kenny): make sure Int64 & Uint64 type can be copied here
obj[name] = message[name];
}
} else if (def.fields[i].type.isArray && def.fields[i].type.type === 'Constants') {
// For a constants array, because its field is empty we just return an empty array here.
obj[name] = [];
} else {
// Proceed further
obj[name] = toPlainObject(message[name], enableTypedArray);
Expand Down
55 changes: 46 additions & 9 deletions rosidl_gen/templates/message.dot
View file
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,11 @@ class {{=objectWrapper}} {
{{? field.default_value !== null && field.type.isPrimitiveType && isTypedArrayType(field.type)}}
this._wrapperFields.{{=field.name}}.fill({{=getTypedArrayName(field.type)}}.from({{=JSON.stringify(field.default_value)}}));
{{?}}
{{? field.type.type === 'string' && field.type.isFixedSizeArray}}
for (let i = 0; i < {{=field.type.arraySize}}; i++) {
primitiveTypes.initString(this._refObject.{{=field.name}}[i]);
}
{{?}}
{{?? !field.type.isPrimitiveType || (field.type.type === 'string' && it.spec.msgName !== 'String')}}
this._wrapperFields.{{=field.name}} = new {{=getWrapperNameByType(field.type)}}();
{{?? it.spec.msgName === 'String'}}
Expand Down Expand Up @@ -376,10 +381,26 @@ class {{=objectWrapper}} {
{{~}}

{{~ it.spec.fields :field}}
{{? field.type.isArray && field.type.isPrimitiveType && field.type.isFixedSizeArray}}
for (let i = 0; i < this._wrapperFields.{{=field.name}}.data.length; i++) {
this._refObject.{{=field.name}}[i] = this._wrapperFields.{{=field.name}}.data[i];
{{? field.type.isArray && field.type.isPrimitiveType && !isTypedArrayType(field.type) && field.type.isFixedSizeArray }}
{{? field.type.type === 'string'}}
for (let i = 0; i < {{=field.type.arraySize}}; i++) {
if (own) {
primitiveTypes.initString(this._refObject.{{=field.name}}[i].ref(), own);
} else {
if (this._{{=field.name}}Array.length === {{=field.type.arraySize}}) {
const value = this._{{=field.name}}Array[i];
this._refObject.{{=field.name}}[i].data = value;
this._refObject.{{=field.name}}[i].size = Buffer.byteLength(value);
this._refObject.{{=field.name}}[i].capacity = Buffer.byteLength(value) + 1;
}
}
}
// For non-typed array like int64/uint64/bool.
{{?? true}}
this._refObject.{{=field.name}} = this._{{=field.name}}Array;
{{?}}
{{?? field.type.isArray && field.type.isPrimitiveType && isTypedArrayType(field.type) && field.type.isFixedSizeArray}}
this._refObject.{{=field.name}} = Array.from(this._wrapperFields.{{=field.name}}.data);
{{?? field.type.isArray && field.type.isPrimitiveType && !isTypedArrayType(field.type)}}
if (!own) {
this._wrapperFields.{{=field.name}}.fill(this._{{=field.name}}Array);
Expand All @@ -391,9 +412,11 @@ class {{=objectWrapper}} {
this._refObject.{{=field.name}} = this._wrapperFields.{{=field.name}}.refObject;
}
{{?? field.type.isArray && !field.type.isPrimitiveType && field.type.isFixedSizeArray}}
for (let i = 0; i < this._wrapperFields.{{=field.name}}.data.length; i++) {
this._refObject.{{=field.name}}[i] = this._wrapperFields.{{=field.name}}.data[i].freeze(own, checkConsistency);
this._refObject.{{=field.name}}[i] = this._wrapperFields.{{=field.name}}.data[i].refObject;
for (let i = 0; i < {{=field.type.arraySize}}; i++) {
if (this._wrapperFields.{{=field.name}}.data[i]) {
this._wrapperFields.{{=field.name}}.data[i].freeze(own, checkConsistency);
this._refObject.{{=field.name}}[i] = this._wrapperFields.{{=field.name}}.data[i].refObject;
}
}
{{?? !field.type.isPrimitiveType || field.type.isArray}}
this._wrapperFields.{{=field.name}}.freeze(own, checkConsistency);
Expand Down Expand Up @@ -429,9 +452,17 @@ class {{=objectWrapper}} {
this._{{=field.name}}Intialized = true;
{{?}}

{{? field.type.isArray && field.type.isPrimitiveType && field.type.isFixedSizeArray}}
this._refObject.{{=field.name}} = refObject.{{=field.name}};
{{? field.type.isArray && field.type.isPrimitiveType && field.type.isFixedSizeArray && isTypedArrayType(field.type)}}
this._wrapperFields.{{=field.name}}.fill(refObject.{{=field.name}}.toArray());
{{?? field.type.isArray && field.type.isPrimitiveType && field.type.isFixedSizeArray && !isTypedArrayType(field.type)}}
{{? field.type.type === 'string' }}
for (let index = 0; index < {{=field.type.arraySize}}; index++) {
this._{{=field.name}}Array[index] = refObject.{{=field.name}}[index].data;
}
// For non-typed array like int64/uint64/bool.
{{?? true}}
this._{{=field.name}}Array = refObject.{{=field.name}}.toArray();
{{?}}
{{?? field.type.isArray && field.type.isPrimitiveType && !isTypedArrayType(field.type)}}
refObject.{{=field.name}}.data.length = refObject.{{=field.name}}.size;
for (let index = 0; index < refObject.{{=field.name}}.size; index++) {
Expand Down Expand Up @@ -472,6 +503,10 @@ class {{=objectWrapper}} {
for (let i = 0; i < {{=field.type.arraySize}}; i++) {
{{=getWrapperNameByType(field.type)}}.freeStruct(refObject.{{=field.name}}[i]);
}
{{?? field.type.isArray && field.type.type === 'string' && field.type.isFixedSizeArray}}
for (let i = 0; i < {{=field.type.arraySize}}; i++) {
{{=getWrapperNameByType(field.type)}}.freeStruct(refObject.{{=field.name}}[i]);
}
{{?? !field.type.isPrimitiveType || (field.type.type === 'string' && it.spec.msgName !== 'String')}}
{{=getWrapperNameByType(field.type)}}.freeStruct(refObject.{{=field.name}});
{{?? it.spec.msgName === 'String'}}
Expand Down Expand Up @@ -502,7 +537,9 @@ class {{=objectWrapper}} {

{{~ it.spec.fields :field}}
get {{=field.name}}() {
{{? field.type.isArray && isTypedArrayType(field.type)}}
{{? field.type.isArray && field.type.type === 'Constants'}}
return [];
{{?? field.type.isArray && isTypedArrayType(field.type)}}
return this._wrapperFields['{{=field.name}}'].data;
{{?? field.type.isArray && field.type.isPrimitiveType}}
return this._{{=field.name}}Array;
Expand Down
121 changes: 119 additions & 2 deletions test/test-fixed-array.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ describe('Test message which has a fixed array of 36', function () {
const node = rclnodejs.createNode('set_map_client');
const client = node.createClient('nav_msgs/srv/SetMap', 'set_map');
assert.throws(() => {
client.sendRequest(mapData, (response) => {});
client.sendRequest(mapData, (response) => { });
}, RangeError);
node.destroy();
done();
Expand All @@ -126,9 +126,126 @@ describe('Test message which has a fixed array of 36', function () {
const node = rclnodejs.createNode('set_map_client');
const client = node.createClient('nav_msgs/srv/SetMap', 'set_map');
assert.throws(() => {
client.sendRequest(mapData, (response) => {});
client.sendRequest(mapData, (response) => { });
}, RangeError);
node.destroy();
done();
});

it('Tested with different kinds of fixed array', function (done) {
const bacicType = {
bool_value: true,
byte_value: 127,
char_value: 97,
float32_value: 1.25,
float64_value: 2.2,
int8_value: 1,
uint8_value: 2,
int16_value: 2,
uint16_value: 1,
int32_value: 1,
uint32_value: 1,
int64_value: 2,
uint64_value: 2,
};

const defaultValue = {
bool_value: true,
byte_value: 50,
char_value: 100,
float32_value: 1.125,
float64_value: 1.125,
int8_value: -50,
uint8_value: 200,
int16_value: -1000,
uint16_value: 2000,
int32_value: -30000,
uint32_value: 60000,
int64_value: -40000000,
uint64_value: 50000000,
};

const msg = {
bool_values: [true, false, true],
byte_values: Uint8Array.from([127, 125, 100]),
char_values: Int8Array.from([127, 125, 100]),
float32_values: Float32Array.from([1.1, 2.2, 3.3]),
float64_values: Float64Array.from([1.1, 2.2, 3.3]),
int8_values: Int8Array.from([1, 2, 3]),
uint8_values: Uint8Array.from([1, 2, 3]),
int16_values: Int16Array.from([1, 2, 3]),
uint16_values: Uint16Array.from([1, 2, 3]),
int32_values: Int32Array.from([1, 2, 3]),
uint32_values: Uint32Array.from([1, 2, 3]),
int64_values: [1, 2, 3],
uint64_values: [1, 2, 3],
string_values: ['hello', 'world', 'abc'],
basic_types_values: [bacicType, bacicType, bacicType],
defaults_values: [defaultValue, defaultValue, defaultValue],
// Use a string, '18446744073709551615', representing UINT64_MAX for ref,
// see details https://github.com/node-ffi-napi/ref-napi/blob/latest/test/uint64.js#L17.
uint64_values_default: [0, 1, '18446744073709551615'],
alignment_check: 100,
};

// Arrays.msg defined https://github.com/ros2/test_interface_files/blob/rolling/msg/Arrays.msg
const expected = {
bool_values: [true, false, true],
byte_values: Uint8Array.from([127, 125, 100]),
char_values: Int8Array.from([127, 125, 100]),
float32_values: Float32Array.from([1.1, 2.2, 3.3]),
float64_values: Float64Array.from([1.1, 2.2, 3.3]),
int8_values: Int8Array.from([1, 2, 3]),
uint8_values: Uint8Array.from([1, 2, 3]),
int16_values: Int16Array.from([1, 2, 3]),
uint16_values: Uint16Array.from([1, 2, 3]),
int32_values: Int32Array.from([1, 2, 3]),
uint32_values: Uint32Array.from([1, 2, 3]),
int64_values: [1, 2, 3],
uint64_values: [1, 2, 3],
string_values: ['hello', 'world', 'abc'],
basic_types_values: [bacicType, bacicType, bacicType],
defaults_values: [defaultValue, defaultValue, defaultValue],
bool_values_default: [false, true, false],
byte_values_default: Uint8Array.from([0, 1, 255]),
char_values_default: Int8Array.from([0, 1, 127]),
float32_values_default: Float32Array.from([1.125, 0.0, -1.125]),
float64_values_default: Float64Array.from([3.1415, 0.0, -3.1415]),
int8_values_default: Int8Array.from([0, 127, -128]),
uint8_values_default: Uint8Array.from([0, 1, 255]),
int16_values_default: Int16Array.from([0, 32767, -32768]),
uint16_values_default: Uint16Array.from([0, 1, 65535]),
int32_values_default: Int32Array.from([0, 2147483647, -2147483648]),
uint32_values_default: Uint32Array.from([0, 1, 4294967295]),
int64_values_default: [0, 9223372036854775807, -9223372036854775808],
uint64_values_default: [0, 1, '18446744073709551615'],
string_values_default: ["", "max value", "min value"],
alignment_check: 100,
constants_values: [],
};

const node = rclnodejs.createNode('fixed_arrays');
let publisher = node.createPublisher(
'test_msgs/msg/Arrays',
'fixed_arrays'
);
let timer = setInterval(() => {
assert.doesNotThrow(() => {
publisher.publish(msg);
}, RangeError);
}, 100);

node.createSubscription(
'test_msgs/msg/Arrays',
'fixed_arrays',
(response) => {
clearInterval(timer);
assert.deepEqual(response, expected);
node.destroy();
done();
}
);

rclnodejs.spin(node);
});
});

0 comments on commit f9ea71b

Please sign in to comment.