Skip to content

Commit

Permalink
Levrage BigInt to represent int64/uint64 (#1030)
Browse files Browse the repository at this point in the history
Due to [double precision floating point format](https://en.wikipedia.org/wiki/Double_precision_floating-point_format),  JavaScript can only safely represent integers [between](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/MAX_SAFE_INTEGER) -(2^53 – 1) to 2^53 – 1, meanwhile, `Number.MAX_SAFE_INTEGER` is defined to represent the maximum safe integer in JavaScript, which is 9007199254740991. Per ROS2 message types, it has [int64](https://github.com/ros2/common_interfaces/blob/rolling/std_msgs/msg/Int64.msg)/[uint64](https://github.com/ros2/common_interfaces/blob/rolling/std_msgs/msg/UInt64.msg), which may be out of the range above.

For current implementation, we leverage [ref](https://github.com/node-ffi-napi/ref-napi) to read/write values from `int64_t` and `uint64_t` data of C++, which can be type of:
1. `number`: when value in [`-Number.MAX_SAFE_INTEGER`  `Number.MAX_SAFE_INTEGER`].
2. `string`: otherwise.

This brings the problem that the type of int64/uint64 be ununified in JavaScript. Along with [BigInt](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/BigInt) introduced, it becomes possible to make the type consistent.

This patch implements to leverage `BigInt` to represent int64/uint64, including:
1. Update `message.dot` to use `BigInt` when hitting int64/uint64.
2. Update `rosidl_parser.js` to convert  int64/uint64 to string in JSON object.
3. Update `message_translator.js` to add `bigint` as primitive.
4. Update `parameter.js` to use `BigInt` for integer and its `ts` definition `parameter.d.ts`.
5. Update tests targeting on messages that include int64/uint64.

Fix: #836, #1025
  • Loading branch information
minggangw authored Feb 14, 2025
1 parent 6295ef8 commit 8d0b4bf
Show file tree
Hide file tree
Showing 23 changed files with 264 additions and 296 deletions.
62 changes: 34 additions & 28 deletions lib/parameter.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ const rclnodejs = require('bindings')('rclnodejs');
const DEFAULT_NUMERIC_RANGE_TOLERANCE = 1e-6;

const PARAMETER_SEPARATOR = '.';
const PARAMETER_BYTE = 10;

/**
* Enum for ParameterType
Expand Down Expand Up @@ -125,7 +126,9 @@ class Parameter {
constructor(name, type, value) {
this._name = name;
this._type = type;
this._value = value;
// Convert to bigint if it's type of `PARAMETER_INTEGER`.
this._value =
this._type == ParameterType.PARAMETER_INTEGER ? BigInt(value) : value;
this._isDirty = true;

this.validate();
Expand Down Expand Up @@ -240,10 +243,10 @@ class Parameter {
msg.double_array_value = this.value;
break;
case ParameterType.PARAMETER_INTEGER:
msg.integer_value = Math.trunc(this.value);
msg.integer_value = this.value;
break;
case ParameterType.PARAMETER_INTEGER_ARRAY:
msg.integer_array_value = this.value.map((val) => Math.trunc(val));
msg.integer_array_value = this.value;
break;
case ParameterType.PARAMETER_STRING:
msg.string_value = this.value;
Expand Down Expand Up @@ -537,10 +540,10 @@ class Range {

/**
* Determine if a value is within this range.
* A TypeError is thrown when value is not a number.
* A TypeError is thrown when value is not a number or bigint.
* Subclasses should override and call this method for basic type checking.
*
* @param {number} value - The number to check.
* @param {number|bigint} value - The number or bigint to check.
* @return {boolean} - True if value satisfies the range; false otherwise.
*/
inRange(value) {
Expand All @@ -550,8 +553,8 @@ class Range {
(inRange, val) => inRange && this.inRange(val),
true
);
} else if (typeof value !== 'number') {
throw new TypeError('Value must be a number');
} else if (typeof value !== 'number' && typeof value !== 'bigint') {
throw new TypeError('Value must be a number or bigint');
}

return true;
Expand Down Expand Up @@ -652,27 +655,16 @@ class FloatingPointRange extends Range {
* Defines a range for integer values.
* @class
*/
class IntegerRange extends FloatingPointRange {
class IntegerRange extends Range {
/**
* Create a new instance.
* @constructor
* @param {number} fromValue - The lowest inclusive value in range
* @param {number} toValue - The highest inclusive value in range
* @param {number} step - The internal unit size.
* @param {number} tolerance - The plus/minus tolerance for number equivalence.
* @param {bigint} fromValue - The lowest inclusive value in range
* @param {bigint} toValue - The highest inclusive value in range
* @param {bigint} step - The internal unit size.
*/
constructor(
fromValue,
toValue,
step = 1,
tolerance = DEFAULT_NUMERIC_RANGE_TOLERANCE
) {
super(
Math.trunc(fromValue),
Math.trunc(toValue),
Math.trunc(step),
tolerance
);
constructor(fromValue, toValue, step = 1n) {
super(fromValue, toValue, step);
}

/**
Expand All @@ -683,12 +675,23 @@ class IntegerRange extends FloatingPointRange {
*/
isValidType(parameterType) {
const result =
parameterType === ParameterType.PARAMETER_BYTE ||
parameterType === ParameterType.PARAMETER_BYTE_ARRAY ||
parameterType === ParameterType.PARAMETER_INTEGER ||
parameterType === ParameterType.PARAMETER_INTEGER_ARRAY;
return result;
}

inRange(value) {
const min = this.fromValue;
const max = this.toValue;
if (value < min || value > max) {
return false;
}

if (this.step != 0n && (value - min) % this.step !== 0n) {
return false;
}
return true;
}
}

/**
Expand Down Expand Up @@ -763,10 +766,13 @@ function validValue(value, type) {
case ParameterType.PARAMETER_STRING:
result = typeof value === 'string';
break;
case ParameterType.PARAMETER_INTEGER:
case ParameterType.PARAMETER_DOUBLE:
case PARAMETER_BYTE:
result = typeof value === 'number';
break;
case ParameterType.PARAMETER_INTEGER:
result = typeof value === 'bigint';
break;
case ParameterType.PARAMETER_BOOL_ARRAY:
case ParameterType.PARAMETER_BYTE_ARRAY:
case ParameterType.PARAMETER_INTEGER_ARRAY:
Expand All @@ -789,7 +795,7 @@ function _validArray(values, type) {
if (type === ParameterType.PARAMETER_BOOL_ARRAY) {
arrayElementType = ParameterType.PARAMETER_BOOL;
} else if (type === ParameterType.PARAMETER_BYTE_ARRAY) {
arrayElementType = ParameterType.PARAMETER_INTEGER;
arrayElementType = PARAMETER_BYTE;
}
if (type === ParameterType.PARAMETER_INTEGER_ARRAY) {
arrayElementType = ParameterType.PARAMETER_INTEGER;
Expand Down
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@
"dot": "^1.1.3",
"dtslint": "^4.2.1",
"fs-extra": "^11.2.0",
"json-bigint": "^1.0.0",
"int64-napi": "^1.0.2",
"is-close": "^1.3.3",
"mkdirp": "^3.0.1",
Expand Down
21 changes: 17 additions & 4 deletions rosidl_gen/message_translator.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,12 @@ function copyMsgObject(msg, obj) {
for (let i in obj) {
if (msg.hasMember(i)) {
const type = typeof obj[i];
if (type === 'string' || type === 'number' || type === 'boolean') {
if (
type === 'string' ||
type === 'number' ||
type === 'boolean' ||
type === 'bigint'
) {
// A primitive-type value
msg[i] = obj[i];
} else if (Array.isArray(obj[i]) || isTypedArray(obj[i])) {
Expand Down Expand Up @@ -80,17 +85,20 @@ function verifyMessage(message, obj) {
case 'char':
case 'int16':
case 'int32':
case 'int64':
case 'byte':
case 'uint16':
case 'uint32':
case 'uint64':
case 'float32':
case 'float64':
if (typeof obj[name] != 'number') {
return false;
}
break;
case 'int64':
case 'uint64':
if (typeof obj[name] != 'bigint') {
return false;
}
case 'bool':
if (typeof obj[name] != 'boolean') {
return false;
Expand Down Expand Up @@ -171,7 +179,12 @@ function toROSMessage(TypeClass, obj) {

function constructFromPlanObject(msg, obj) {
const type = typeof obj;
if (type === 'string' || type === 'number' || type === 'boolean') {
if (
type === 'string' ||
type === 'number' ||
type === 'boolean' ||
type === 'bigint'
) {
msg.data = obj;
} else if (type === 'object') {
copyMsgObject(msg, obj);
Expand Down
8 changes: 2 additions & 6 deletions rosidl_gen/packages.js
Original file line number Diff line number Diff line change
Expand Up @@ -161,9 +161,7 @@ async function generateMsgForSrv(filePath, interfaceInfo, pkgMap) {
async function addInterfaceInfos(filePath, dir, pkgMap) {
const interfaceInfo = grabInterfaceInfo(filePath, true);
const ignore = pkgFilters.matchesAny(interfaceInfo);
if (ignore) {
console.log('Omitting filtered interface: ', interfaceInfo);
} else {
if (!ignore) {
if (path.extname(filePath) === '.msg') {
// Some .msg files were generated prior to 0.3.2 for .action files,
// which has been disabled. So these files should be ignored here.
Expand Down Expand Up @@ -232,9 +230,7 @@ async function findPackagesInDirectory(dir) {
amentExecuted
);
const ignore = pkgFilters.matchesAny(interfaceInfo);
if (ignore) {
console.log('Omitting filtered interface: ', interfaceInfo);
} else {
if (!ignore) {
if (path.extname(file.name) === '.msg') {
// Some .msg files were generated prior to 0.3.2 for .action files,
// which has been disabled. So these files should be ignored here.
Expand Down
24 changes: 22 additions & 2 deletions rosidl_gen/templates/message.dot
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,10 @@ function isTypedArrayType(type) {
return typedArrayType.indexOf(type.type.toLowerCase()) !== -1;
}

function isBigInt(type) {
return ['int64', 'uint64'].indexOf(type.type.toLowerCase()) !== -1;
}

const willUseTypedArray = isTypedArrayType(it.spec.baseType);
const currentTypedArray = getTypedArrayName(it.spec.baseType);
const currentTypedArrayElementType = getTypedArrayElementName(it.spec.baseType);
Expand Down Expand Up @@ -303,7 +307,13 @@ class {{=objectWrapper}} {
this._refObject.{{=field.name}} = {{=field.default_value}};
{{?}}
{{?? field.type.isPrimitiveType && !isTypedArrayType(field.type) && field.default_value}}
this._{{=field.name}}Array = {{=JSON.stringify(field.default_value)}};
{{? isBigInt(field.type)}}
{{/* For non-TypedArray like int64/uint64. */}}
this._{{=field.name}}Array = {{=JSON.stringify(field.default_value)}}.map(num => BigInt(num));
{{??}}
{{/* For non-TypedArray like bool. */}}
this._{{=field.name}}Array = {{=JSON.stringify(field.default_value)}};
{{?}}
{{?? field.type.isPrimitiveType && isTypedArrayType(field.type) && field.default_value}}
this._wrapperFields.{{=field.name}}.fill({{=getTypedArrayName(field.type)}}.from({{=JSON.stringify(field.default_value)}}));
{{?}}
Expand Down Expand Up @@ -376,8 +386,11 @@ class {{=objectWrapper}} {
}
}
}
{{?? isBigInt(field.type)}}
{{/* For non-TypedArray like int64/uint64. */}}
this._refObject.{{=field.name}} = this._{{=field.name}}Array.map(num => num.toString());
{{??}}
{{/* For non-TypedArray like int64/uint64/bool. */}}
{{/* For non-TypedArray like bool. */}}
this._refObject.{{=field.name}} = this._{{=field.name}}Array;
{{?}}
{{?? field.type.isArray && field.type.isPrimitiveType && isTypedArrayType(field.type) && field.type.isFixedSizeArray}}
Expand Down Expand Up @@ -527,6 +540,8 @@ class {{=objectWrapper}} {
return this._wrapperFields.{{=field.name}};
{{?? !field.type.isArray && field.type.type === 'string' && it.spec.msgName !== 'String'}}
return this._wrapperFields.{{=field.name}}.data;
{{?? isBigInt(field.type)}}
return BigInt(this._refObject.{{=field.name}});
{{??}}
return this._refObject.{{=field.name}};
{{?}}
Expand Down Expand Up @@ -559,6 +574,11 @@ class {{=objectWrapper}} {
}
{{?? !field.type.isArray && field.type.type === 'string' && it.spec.msgName !== 'String'}}
this._wrapperFields.{{=field.name}}.data = value;
{{?? isBigInt(field.type)}}
if (typeof value !== "bigint") {
throw new TypeError('{{=field.name}} must be type of bigint');
}
this._refObject.{{=field.name}} = value.toString();
{{??}}
{{? it.spec.msgName === 'String'}}
this._refObject.size = Buffer.byteLength(value);
Expand Down
30 changes: 29 additions & 1 deletion rosidl_parser/rosidl_parser.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,20 @@

'use strict';

const compareVersions = require('compare-versions');
const path = require('path');
const execFile = require('child_process').execFile;

const pythonExecutable = require('./py_utils').getPythonExecutable('python3');

const contextSupportedVersion = '21.0.0.0';
const currentVersion = process.version;
const isContextSupported = compareVersions.compare(
currentVersion.substring(1, currentVersion.length),
contextSupportedVersion,
'>='
);

const rosidlParser = {
parseMessageFile(packageName, filePath) {
return this._parseFile('parse_message_file', packageName, filePath);
Expand All @@ -32,6 +41,25 @@ const rosidlParser = {
return this._parseFile('parse_action_file', packageName, filePath);
},

_parseJSONObject(str) {
// For nodejs >= `contextSupportedVersion`, we leverage context parameter to
// convert unsafe integer to string, otherwise, json-bigint is used.
// See https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/JSON/parse
if (isContextSupported) {
return JSON.parse(str, (key, value, context) => {
if (
Number.isInteger(value) &&
!Number.isSafeInteger(Number(context.source))
) {
return context.source;
}
return value;
});
}
const JSONbigString = require('json-bigint')({ storeAsString: true });
return JSONbigString.parse(str);
},

_parseFile(command, packageName, filePath) {
return new Promise((resolve, reject) => {
const args = [
Expand All @@ -54,7 +82,7 @@ const rosidlParser = {
)
);
} else {
resolve(JSON.parse(stdout));
resolve(this._parseJSONObject(stdout));
}
}
);
Expand Down
6 changes: 4 additions & 2 deletions rostsd_gen/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -478,7 +478,6 @@ function primitiveType2JSName(type) {
case 'int8':
case 'int16':
case 'int32':
case 'int64':

// signed explicit float types
case 'float32':
Expand All @@ -488,7 +487,6 @@ function primitiveType2JSName(type) {
case 'uint8':
case 'uint16':
case 'uint32':
case 'uint64':
jsName = 'number';
break;
case 'bool':
Expand All @@ -499,6 +497,10 @@ function primitiveType2JSName(type) {
case 'wstring':
jsName = 'string';
break;
case 'int64':
case 'uint64':
jsName = 'bigint';
break;
}

return jsName;
Expand Down
6 changes: 3 additions & 3 deletions test/client_setup.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,13 @@ rclnodejs
const Int8 = 'std_msgs/msg/Int8';
var client = node.createClient(AddTwoInts, 'add_two_ints');
const request = {
a: 1,
b: 2,
a: 1n,
b: 2n,
};
var publisher = node.createPublisher(Int8, 'back_add_two_ints');
client.waitForService().then(() => {
client.sendRequest(request, (response) => {
publisher.publish(response.sum);
publisher.publish(Number(response.sum));
});
});

Expand Down
4 changes: 4 additions & 0 deletions test/publisher_msg.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@ const rclnodejs = require('../index.js');
var rclType = process.argv[2];
var rclValue = eval(process.argv[3]);

if (['int64', 'uint64'].indexOf(rclType.toLowerCase()) !== -1) {
rclValue = BigInt(rclValue);
}

rclnodejs
.init()
.then(() => {
Expand Down
Loading

0 comments on commit 8d0b4bf

Please sign in to comment.