Add support for sending messages from NUsight#232
Conversation
TrentHouliston
left a comment
There was a problem hiding this comment.
The parser still needs a little work, and it would be good to make the API consistent for sending regardless of where it's sent from
src/client/network/network.ts
Outdated
| /** | ||
| * Send the given message on the network | ||
| */ | ||
| send(message: Message, target?: string): void { |
There was a problem hiding this comment.
Why make a new Message type with a target as a 2nd parameter here over using NUClearNetSend?
There was a problem hiding this comment.
Changed to accept a NUClearNetSend.
| import * as XXH from 'xxhashjs' | ||
|
|
||
| import { createSingletonFactory } from '../../shared/base/create_singleton_factory' | ||
| import { hashType } from '../../shared/nuclearnet/nuclearnet_proxy_parser' |
There was a problem hiding this comment.
I don't know where this function should go, both of these places seem like a bad spot.
How do we not have a utility location yet :P
There was a problem hiding this comment.
How do we not have a utility location yet :P
NEVER!!! 🙂
| case 'packet': { | ||
| const { id, data: [key, { target, type, payload, reliable }] } = packet | ||
| return callback([ | ||
| JSON.stringify({ id, nsp, key, header: { target, type, reliable } }), |
There was a problem hiding this comment.
You're sending type as a string here, and adding a hashType below that isn't a part of a NUClearNetSend packet.
The decoding logic below will take that and add a .hash element to it afterwards that'll be ignored, so the hashType(type) here is doing nothing.
However, NUClearNetSend does accept a buffer or a string here, so maybe instead don't send the type in json and in the parser below add it as the type.
There was a problem hiding this comment.
Updated the decoder to not add the unused hash.
This adds support for sending messages from NUsight onto the network.
Added a new method
send(message: Message, target?: string): voidonsrc/client/network/network.ts, used by components in their local network file.Informally tested using a simulator in #208.