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

Levrage BigInt to represent int64/uint64 #1030

Merged
merged 7 commits into from
Feb 14, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading