Skip to content

Commit

Permalink
Add support for TypedArrays in User Scripts using @foxglove/schemas (…
Browse files Browse the repository at this point in the history
…#6890)

**User-Facing Changes**
Fixed a type incompatibility issue with User Scripts that import from
@foxglove/schemas.

**Description**
Fixes #6821 
Fixes FG-4976
Depends on foxglove/schemas#128
Also updated JSON message datatype extraction to treat integer arrays as
`Uint32Array` rather than `Float64Array`; this fixed a similar issue as
#6821 with an example json-based mcap I had lying around.
  • Loading branch information
jtbandes authored and pezy committed Oct 25, 2023
1 parent 50ae516 commit 593ca28
Show file tree
Hide file tree
Showing 9 changed files with 110 additions and 21 deletions.
2 changes: 1 addition & 1 deletion benchmark/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
"@foxglove/den": "workspace:*",
"@foxglove/log": "workspace:*",
"@foxglove/rostime": "1.1.2",
"@foxglove/schemas": "1.5.1",
"@foxglove/schemas": "1.6.0",
"@foxglove/studio": "workspace:*",
"@foxglove/tsconfig": "2.0.0",
"@pmmmwh/react-refresh-webpack-plugin": "0.5.11",
Expand Down
2 changes: 1 addition & 1 deletion packages/mcap-support/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
"@foxglove/rosmsg": "4.2.2",
"@foxglove/rosmsg-serialization": "2.0.2",
"@foxglove/rosmsg2-serialization": "2.0.2",
"@foxglove/schemas": "1.5.1",
"@foxglove/schemas": "1.6.0",
"@foxglove/wasm-bz2": "0.1.1",
"@foxglove/wasm-lz4": "1.0.2",
"@foxglove/wasm-zstd": "1.0.1",
Expand Down
12 changes: 10 additions & 2 deletions packages/mcap-support/src/parseJsonSchema.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ describe("parseJsonSchema", () => {
bin: { type: "string", contentEncoding: "base64" },
num: { type: "number" },
int: { type: "integer" },
uint: { type: "integer", minimum: 0 },
uint2: { type: "integer", exclusiveMinimum: 0 },
bool: { type: "boolean" },
},
},
Expand All @@ -64,7 +66,9 @@ describe("parseJsonSchema", () => {
{ name: "str", type: "string" },
{ name: "bin", type: "uint8", isArray: true },
{ name: "num", type: "float64" },
{ name: "int", type: "float64" },
{ name: "int", type: "int32" },
{ name: "uint", type: "uint32" },
{ name: "uint2", type: "uint32" },
{ name: "bool", type: "bool" },
],
},
Expand Down Expand Up @@ -94,6 +98,8 @@ describe("parseJsonSchema", () => {
str: { type: "array", items: { type: "string" } },
num: { type: "array", items: { type: "number" } },
int: { type: "array", items: { type: "integer" } },
uint: { type: "array", items: { type: "integer", minimum: 0 } },
uint2: { type: "array", items: { type: "integer", exclusiveMinimum: 0 } },
bool: { type: "array", items: { type: "boolean" } },
},
},
Expand All @@ -104,7 +110,9 @@ describe("parseJsonSchema", () => {
definitions: [
{ name: "str", type: "string", isArray: true },
{ name: "num", type: "float64", isArray: true },
{ name: "int", type: "float64", isArray: true },
{ name: "int", type: "int32", isArray: true },
{ name: "uint", type: "uint32", isArray: true },
{ name: "uint2", type: "uint32", isArray: true },
{ name: "bool", type: "bool", isArray: true },
],
},
Expand Down
25 changes: 23 additions & 2 deletions packages/mcap-support/src/parseJsonSchema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -96,9 +96,19 @@ export function parseJsonSchema(
}
break;
case "number":
case "integer":
fields.push({ name: fieldName, type: "float64" });
break;
case "integer":
fields.push({
name: fieldName,
type:
(typeof fieldSchema.minimum === "number" && fieldSchema.minimum >= 0) ||
(typeof fieldSchema.exclusiveMinimum === "number" &&
fieldSchema.exclusiveMinimum >= 0)
? "uint32"
: "int32",
});
break;
case "object": {
const nestedTypeName = `${typeName}.${fieldName}`;
const postprocessNestedObject = addFieldsRecursive(
Expand Down Expand Up @@ -134,9 +144,20 @@ export function parseJsonSchema(
fields.push({ name: fieldName, type: "string", isArray: true });
break;
case "number":
case "integer":
fields.push({ name: fieldName, type: "float64", isArray: true });
break;
case "integer":
fields.push({
name: fieldName,
type:
(typeof itemSchema.minimum === "number" && itemSchema.minimum >= 0) ||
(typeof itemSchema.exclusiveMinimum === "number" &&
itemSchema.exclusiveMinimum >= 0)
? "uint32"
: "int32",
isArray: true,
});
break;
case "object": {
const nestedTypeName = `${typeName}.${fieldName}`;
const postprocessArrayItem = addFieldsRecursive(
Expand Down
2 changes: 1 addition & 1 deletion packages/studio-base/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@
"@foxglove/rosmsg-serialization": "2.0.2",
"@foxglove/rosmsg2-serialization": "2.0.2",
"@foxglove/rostime": "1.1.2",
"@foxglove/schemas": "1.5.1",
"@foxglove/schemas": "1.6.0",
"@foxglove/studio": "workspace:*",
"@foxglove/theme": "workspace:*",
"@foxglove/three-text": "0.2.2",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -295,7 +295,7 @@ describe("getMessagePathSearchItems", () => {
"suffix": {
"isLeaf": true,
"pathSuffix": ".circles[:].timestamp.sec",
"type": "float64",
"type": "uint32",
},
"topic": {
"name": "annotations",
Expand All @@ -308,7 +308,7 @@ describe("getMessagePathSearchItems", () => {
"suffix": {
"isLeaf": true,
"pathSuffix": ".circles[:].timestamp.nsec",
"type": "float64",
"type": "uint32",
},
"topic": {
"name": "annotations",
Expand Down Expand Up @@ -542,7 +542,7 @@ describe("getMessagePathSearchItems", () => {
"suffix": {
"isLeaf": true,
"pathSuffix": ".points[:].timestamp.sec",
"type": "float64",
"type": "uint32",
},
"topic": {
"name": "annotations",
Expand All @@ -555,7 +555,7 @@ describe("getMessagePathSearchItems", () => {
"suffix": {
"isLeaf": true,
"pathSuffix": ".points[:].timestamp.nsec",
"type": "float64",
"type": "uint32",
},
"topic": {
"name": "annotations",
Expand Down Expand Up @@ -854,7 +854,7 @@ describe("getMessagePathSearchItems", () => {
"suffix": {
"isLeaf": true,
"pathSuffix": ".texts[:].timestamp.sec",
"type": "float64",
"type": "uint32",
},
"topic": {
"name": "annotations",
Expand All @@ -867,7 +867,7 @@ describe("getMessagePathSearchItems", () => {
"suffix": {
"isLeaf": true,
"pathSuffix": ".texts[:].timestamp.nsec",
"type": "float64",
"type": "uint32",
},
"topic": {
"name": "annotations",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1176,6 +1176,63 @@ describe("pipeline", () => {
datatypes: basicDatatypes,
outputDatatype: "foxglove.Color",
},
{
description: "Allows TypedArrays when output datatype is from @foxglove/schemas",
sourceCode: `
import { LaserScan } from "@foxglove/schemas";
export const inputs = [];
export const output = "${DEFAULT_STUDIO_NODE_PREFIX}";
const publisher = (message: any): LaserScan => {
return {
timestamp: {sec: 0, nsec: 1},
frame_id: "",
pose: {
position: {x: 0, y: 0, z: 0},
orientation: {x: 0, y: 0, z: 0, w: 1},
},
start_angle: 0,
end_angle: 0,
ranges: new Float64Array([1, 2, 3]),
intensities: new Float32Array([1, 2, 3]),
};
};
export default publisher;`,
datatypes: basicDatatypes,
outputDatatype: "foxglove.LaserScan",
},
{
description:
"Rejects wrong kind of TypedArray when output datatype is from @foxglove/schemas",
sourceCode: `
import { LaserScan } from "@foxglove/schemas";
export const inputs = [];
export const output = "${DEFAULT_STUDIO_NODE_PREFIX}";
const publisher = (message: any): LaserScan => {
return {
timestamp: {sec: 0, nsec: 1},
frame_id: "",
pose: {
position: {x: 0, y: 0, z: 0},
orientation: {x: 0, y: 0, z: 0, w: 1},
},
start_angle: 0,
end_angle: 0,
ranges: new Float64Array([1, 2, 3]),
intensities: new Uint32Array([1, 2, 3]),
};
};
export default publisher;`,
error: 2322,
errorMessage: expect.stringContaining(
`Type 'Uint32Array' is not assignable to type 'number[] | Float32Array | Float64Array'`,
),
},
{
description: "Should handle deep subtype lookup",
sourceCode: `
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,10 @@ import { UserScriptProjectConfig, UserScriptProjectFile } from "./types";
* Generates virtual ts files for each type exported by the @foxglove/schemas package.
*/
export function generateFoxgloveSchemaDeclarations(): UserScriptProjectFile[] {
const schemas = _.sortBy([...exportTypeScriptSchemas().entries()], ([name]) => name);
const schemas = _.sortBy(
[...exportTypeScriptSchemas({ includeTypedArrays: true }).entries()],
([name]) => name,
);
const files = schemas.map(([name, sourceCode]) => {
return {
fileName: `@foxglove/schemas/${name}.ts`,
Expand Down
14 changes: 7 additions & 7 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -2493,7 +2493,7 @@ __metadata:
"@foxglove/rosmsg": 4.2.2
"@foxglove/rosmsg-serialization": 2.0.2
"@foxglove/rosmsg2-serialization": 2.0.2
"@foxglove/schemas": 1.5.1
"@foxglove/schemas": 1.6.0
"@foxglove/tsconfig": 2.0.0
"@foxglove/wasm-bz2": 0.1.1
"@foxglove/wasm-lz4": 1.0.2
Expand Down Expand Up @@ -2674,13 +2674,13 @@ __metadata:
languageName: node
linkType: hard

"@foxglove/schemas@npm:1.5.1, @foxglove/schemas@npm:^1.3.1":
version: 1.5.1
resolution: "@foxglove/schemas@npm:1.5.1"
"@foxglove/schemas@npm:1.6.0, @foxglove/schemas@npm:^1.3.1":
version: 1.6.0
resolution: "@foxglove/schemas@npm:1.6.0"
dependencies:
"@foxglove/rosmsg-msgs-common": ^3.0.0
tslib: ^2.5.0
checksum: 4dd054a318ead308ce980e51eb50c3bcd1c59f9c94707234f8e293bc9b52f2727a21fdad4516f3c4ba0d19ec8058cd60315fe526b90ed9085d52519aa98d12ad
checksum: ff3552a9d22ed47dd05730735abbab4008502e1db76941605b850bddc65b8376a3a613a9885014b2744b4621901b21ee4d0071f899e89248c04165af0226e8a0
languageName: node
linkType: hard

Expand Down Expand Up @@ -2718,7 +2718,7 @@ __metadata:
"@foxglove/rosmsg-serialization": 2.0.2
"@foxglove/rosmsg2-serialization": 2.0.2
"@foxglove/rostime": 1.1.2
"@foxglove/schemas": 1.5.1
"@foxglove/schemas": 1.6.0
"@foxglove/studio": "workspace:*"
"@foxglove/theme": "workspace:*"
"@foxglove/three-text": 0.2.2
Expand Down Expand Up @@ -8622,7 +8622,7 @@ __metadata:
"@foxglove/den": "workspace:*"
"@foxglove/log": "workspace:*"
"@foxglove/rostime": 1.1.2
"@foxglove/schemas": 1.5.1
"@foxglove/schemas": 1.6.0
"@foxglove/studio": "workspace:*"
"@foxglove/studio-base": "workspace:*"
"@foxglove/tsconfig": 2.0.0
Expand Down

0 comments on commit 593ca28

Please sign in to comment.