Skip to content

Commit 0183a0d

Browse files
committed
Use enormous union type to make message types stricter
Fixes #1069 and a couple other things we hadn't caught yet.
1 parent fd8efab commit 0183a0d

File tree

8 files changed

+139
-108
lines changed

8 files changed

+139
-108
lines changed

src/core/Action.ts

Lines changed: 12 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -82,15 +82,14 @@ export default class Action<
8282
}
8383
});
8484

85-
const call = {
85+
this.ros.callOnConnection({
8686
op: "send_action_goal",
8787
id: actionGoalId,
8888
action: this.name,
8989
action_type: this.actionType,
9090
args: goal,
9191
feedback: true,
92-
};
93-
this.ros.callOnConnection(call);
92+
});
9493

9594
return actionGoalId;
9695
}
@@ -101,12 +100,11 @@ export default class Action<
101100
* @param id - The ID of the action goal to cancel.
102101
*/
103102
cancelGoal(id: string) {
104-
const call = {
103+
this.ros.callOnConnection({
105104
op: "cancel_action_goal",
106105
id: id,
107106
action: this.name,
108-
};
109-
this.ros.callOnConnection(call);
107+
});
110108
}
111109

112110
/**
@@ -200,13 +198,12 @@ export default class Action<
200198
* @param feedback - The feedback to send.
201199
*/
202200
sendFeedback(id: string, feedback: TFeedback) {
203-
const call = {
201+
this.ros.callOnConnection({
204202
op: "action_feedback",
205203
id: id,
206204
action: this.name,
207205
values: feedback,
208-
};
209-
this.ros.callOnConnection(call);
206+
});
210207
}
211208

212209
/**
@@ -216,15 +213,14 @@ export default class Action<
216213
* @param result - The result to set.
217214
*/
218215
setSucceeded(id: string, result: TResult) {
219-
const call = {
216+
this.ros.callOnConnection({
220217
op: "action_result",
221218
id: id,
222219
action: this.name,
223220
values: result,
224221
status: GoalStatus.STATUS_SUCCEEDED,
225222
result: true,
226-
};
227-
this.ros.callOnConnection(call);
223+
});
228224
}
229225

230226
/**
@@ -234,15 +230,14 @@ export default class Action<
234230
* @param result - The result to set.
235231
*/
236232
setCanceled(id: string, result: TResult) {
237-
const call = {
233+
this.ros.callOnConnection({
238234
op: "action_result",
239235
id: id,
240236
action: this.name,
241237
values: result,
242238
status: GoalStatus.STATUS_CANCELED,
243239
result: true,
244-
};
245-
this.ros.callOnConnection(call);
240+
});
246241
}
247242

248243
/**
@@ -252,11 +247,11 @@ export default class Action<
252247
*/
253248
setFailed(id: string) {
254249
const call = {
255-
op: "action_result",
250+
op: "action_result" as const,
256251
id: id,
257252
action: this.name,
258253
status: GoalStatus.STATUS_ABORTED,
259-
result: false,
254+
result: false as const,
260255
};
261256
this.ros.callOnConnection(call);
262257
}

src/core/Ros.ts

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
import type {
77
RosbridgeMessage,
8+
RosbridgeMessageBase,
89
RosbridgeSetStatusLevelMessage,
910
} from "../types/protocol.js";
1011
import {
@@ -121,7 +122,7 @@ export default class Ros extends EventEmitter<
121122
this.transport?.close();
122123
}
123124

124-
private handleMessage(message: RosbridgeMessage) {
125+
private handleMessage(message: RosbridgeMessageBase) {
125126
if (isRosbridgePublishMessage(message)) {
126127
this.emit(message.topic, message);
127128
} else if (isRosbridgeServiceResponseMessage(message)) {
@@ -165,12 +166,12 @@ export default class Ros extends EventEmitter<
165166
client: string,
166167
dest: string,
167168
rand: string,
168-
t: object,
169+
t: number,
169170
level: string,
170-
end: object,
171+
end: number,
171172
) {
172-
// create the request
173-
const auth = {
173+
// send the request
174+
this.callOnConnection({
174175
op: "auth",
175176
mac: mac,
176177
client: client,
@@ -179,19 +180,14 @@ export default class Ros extends EventEmitter<
179180
t: t,
180181
level: level,
181182
end: end,
182-
};
183-
// send the request
184-
this.callOnConnection(auth);
183+
});
185184
}
186185

187186
/**
188187
* Sends the message to the transport.
189188
* If not connected, queues the message to send once reconnected.
190189
*/
191-
// eslint-disable-next-line @typescript-eslint/no-unnecessary-type-parameters -- to broaden argument type to any RosbridgeMessage variant
192-
public callOnConnection<T extends RosbridgeMessage = RosbridgeMessage>(
193-
message: T,
194-
) {
190+
public callOnConnection(message: RosbridgeMessage) {
195191
if (this.isConnected()) {
196192
this.transport?.send(message);
197193
} else {
@@ -207,7 +203,10 @@ export default class Ros extends EventEmitter<
207203
* @param level - Status level (none, error, warning, info).
208204
* @param [id] - Operation ID to change status level on.
209205
*/
210-
public setStatusLevel(level: string, id?: string) {
206+
public setStatusLevel(
207+
level: RosbridgeSetStatusLevelMessage["level"],
208+
id?: string,
209+
) {
211210
const levelMsg: RosbridgeSetStatusLevelMessage = {
212211
op: "set_level",
213212
level,

src/core/Service.ts

Lines changed: 9 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,7 @@
44
*/
55

66
import { EventEmitter } from "eventemitter3";
7-
import type {
8-
RosbridgeMessage,
9-
RosbridgeServiceResponseMessage,
10-
} from "../types/protocol.ts";
7+
import type { RosbridgeMessage } from "../types/protocol.ts";
118
import {
129
isRosbridgeCallServiceMessage,
1310
isRosbridgeServiceResponseMessage,
@@ -88,16 +85,13 @@ export default class Service<TRequest, TResponse> extends EventEmitter {
8885
}
8986
});
9087

91-
const call = {
88+
this.ros.callOnConnection({
9289
op: "call_service",
9390
id: serviceCallId,
9491
service: this.name,
95-
type: this.serviceType,
9692
args: request,
9793
timeout: timeout,
98-
};
99-
100-
this.ros.callOnConnection(call);
94+
});
10195
}
10296
/**
10397
* Advertise the service. This turns the Service object from a client
@@ -125,7 +119,8 @@ export default class Service<TRequest, TResponse> extends EventEmitter {
125119
`Invalid message received on service channel: ${JSON.stringify(rosbridgeRequest)}`,
126120
);
127121
}
128-
const response = {};
122+
// @ts-expect-error -- TypeScript doesn't have a way to handle the out-parameter model used here.
123+
const response: TResponse = {};
129124
let success: boolean;
130125
try {
131126
success = callback(rosbridgeRequest.args, response);
@@ -140,14 +135,14 @@ export default class Service<TRequest, TResponse> extends EventEmitter {
140135
values: response,
141136
result: success,
142137
id: rosbridgeRequest.id,
143-
} satisfies RosbridgeServiceResponseMessage<Partial<TResponse>>);
138+
});
144139
} else {
145140
this.ros.callOnConnection({
146141
op: "service_response",
147142
service: this.name,
148143
result: success,
149144
id: rosbridgeRequest.id,
150-
} satisfies RosbridgeServiceResponseMessage<Partial<TResponse>>);
145+
});
151146
}
152147
};
153148

@@ -247,15 +242,15 @@ export default class Service<TRequest, TResponse> extends EventEmitter {
247242
result: true,
248243
values: await callback(rosbridgeRequest.args),
249244
id: rosbridgeRequest.id,
250-
} satisfies RosbridgeServiceResponseMessage<TResponse>);
245+
});
251246
} catch (err) {
252247
this.ros.callOnConnection({
253248
op: "service_response",
254249
service: this.name,
255250
result: false,
256251
values: String(err),
257252
id: rosbridgeRequest.id,
258-
} satisfies RosbridgeServiceResponseMessage<TResponse>);
253+
});
259254
}
260255
})().catch(console.error);
261256
};

src/core/Topic.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -258,14 +258,12 @@ export default class Topic<T> extends EventEmitter<{
258258
this.advertise();
259259
}
260260

261-
const call = {
261+
this.ros.callOnConnection({
262262
op: "publish",
263263
id: `publish:${this.name}:${uuidv4()}`,
264264
topic: this.name,
265265
msg: message,
266-
latch: this.latch,
267-
};
268-
this.ros.callOnConnection(call);
266+
});
269267
}
270268

271269
/**

src/core/transport/Transport.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import type {
33
RosbridgePngMessage,
44
RosbridgeMessage,
55
RosbridgeFragmentMessage,
6+
RosbridgeMessageBase,
67
} from "../../types/protocol.js";
78
import {
89
isRosbridgeFragmentMessage,
@@ -60,7 +61,7 @@ export abstract class AbstractTransport
6061
open: [TransportEvent];
6162
close: [TransportEvent];
6263
error: [TransportEvent];
63-
message: [RosbridgeMessage];
64+
message: [RosbridgeMessageBase];
6465
}>
6566
implements ITransport
6667
{
@@ -114,7 +115,7 @@ export abstract class AbstractTransport
114115
* If the message is a PNG, it is decompressed and reprocessed.
115116
* Otherwise, the message is emitted.
116117
*/
117-
private handleRosbridgeMessage(message: RosbridgeMessage) {
118+
private handleRosbridgeMessage(message: RosbridgeMessageBase) {
118119
if (isRosbridgeFragmentMessage(message)) {
119120
this.handleRosbridgeFragmentMessage(message);
120121
} else if (isRosbridgePngMessage(message)) {

0 commit comments

Comments
 (0)