Skip to content
Open
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
57 changes: 48 additions & 9 deletions packages/server-admin-ui/src/views/ServerConfig/BasicProvider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -971,7 +971,10 @@ function BaudRateInputCanboat({
onChange: OnChangeHandler
}) {
// Default baud rate based on device type - controlled with fallback
const defaultBaudrate = value.type === 'ikonvert-canboatjs' ? 230400 : 115200
const defaultBaudrate =
value.type === 'ikonvert-canboatjs' || value.type === 'ikonvert'
? 230400
: 115200
Comment on lines +974 to +977

@coderabbitai coderabbitai Bot Aug 6, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Name the protocol baud-rate defaults.

Raw baud-rate literals hide protocol configuration and make UI and runtime defaults harder to keep aligned.

  • packages/server-admin-ui/src/views/ServerConfig/BasicProvider.tsx#L974-L977: define named baud-rate constants and use them in BaudRateInputCanboat.
  • packages/streams/src/simple.ts#L428-L447: define named native transport baud-rate constants and use them in command construction.
📍 Affects 2 files
  • packages/server-admin-ui/src/views/ServerConfig/BasicProvider.tsx#L974-L977 (this comment)
  • packages/streams/src/simple.ts#L428-L447
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/server-admin-ui/src/views/ServerConfig/BasicProvider.tsx` around
lines 974 - 977, Define named protocol baud-rate constants in
packages/server-admin-ui/src/views/ServerConfig/BasicProvider.tsx and use them
in BaudRateInputCanboat instead of raw literals. Also define named native
transport baud-rate constants in packages/streams/src/simple.ts and use those
constants when constructing the commands in the affected 428-447 range.

Source: Coding guidelines

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coderabbitai Skipped: the baud-rate literals sit in two files with different lifetimes (a React input default and the native command construction), and hoisting them into shared constants would mean a new cross-package export for two values. The UI change above removes the cases where the two could disagree.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dirkwa, acknowledged. The UI and native command paths have separate responsibilities. A shared cross-package constant would add coupling without sufficient benefit. The updated iKonvert condition keeps the relevant UI defaults aligned.

You are interacting with an AI system.

const displayBaudrate = value.baudrate ?? defaultBaudrate

return (
Expand Down Expand Up @@ -1690,13 +1693,25 @@ function NMEA2000({ value, onChange, hasAnalyzer }: TypeComponentProps) {
Actisense NGT-1 (canboat)
</option>
<option value="ikonvert-canboatjs">iKonvert (canboatjs)</option>
<option value="ikonvert" disabled={!hasAnalyzer}>
iKonvert (canboat)
</option>
<option value="navlink2-tcp-canboatjs">NavLink2 (canboatjs)</option>
<option value="navlink2" disabled={!hasAnalyzer}>
NavLink2 (canboat)
</option>
<option value="canboat-csv-canboatjs">
canboat-pipeline CSV R/W (canboatjs)
</option>
<option value="ydwg02-canboatjs">
Yacht Devices RAW TCP (canboatjs)
</option>
<option value="ydwg02" disabled={!hasAnalyzer}>
Yacht Devices RAW TCP (canboat)
</option>
<option value="ydwg02-udp" disabled={!hasAnalyzer}>
Yacht Devices RAW UDP (canboat)
</option>
<option value="ydwg02-udp-canboatjs">
Yacht Devices RAW UDP (canboatjs)
</option>
Expand All @@ -1713,9 +1728,15 @@ function NMEA2000({ value, onChange, hasAnalyzer }: TypeComponentProps) {
<option value="w2k-1-n2k-actisense-canboatjs">
W2K-1 N2K ACTISENSE (canboatjs)
</option>
<option value="w2k-1-ascii" disabled={!hasAnalyzer}>
W2K-1 N2K ASCII (canboat)
</option>
<option value="maretron-ipg-canboatjs">
Maretron IPG 100 (canboatjs)
</option>
<option value="maretron-ipg" disabled={!hasAnalyzer}>
Maretron IPG 100 (canboat)
</option>
<option value="canbus" disabled={!hasAnalyzer}>
Canbus (canboat)
</option>
Expand All @@ -1725,13 +1746,22 @@ function NMEA2000({ value, onChange, hasAnalyzer }: TypeComponentProps) {
{(value.options.type === 'ngt-1' ||
value.options.type === 'ngt-1-canboatjs' ||
value.options.type === 'ydwg02-usb-canboatjs' ||
value.options.type === 'ikonvert-canboatjs') && (
value.options.type === 'ikonvert-canboatjs' ||
value.options.type === 'ikonvert') && (
<div>
<DeviceInput value={value.options} onChange={onChange} />
<BaudRateInputCanboat value={value.options} onChange={onChange} />
{/* The canboatjs iKonvert and YDWG-02 USB elements pin their
protocol's baud rate, so offering the control there would
save a value the server ignores. */}
{value.options.type !== 'ikonvert-canboatjs' &&
value.options.type !== 'ydwg02-usb-canboatjs' && (
<BaudRateInputCanboat value={value.options} onChange={onChange} />
)}
</div>
)}
{value.options.type === 'ydwg02-canboatjs' && (
{(value.options.type === 'ydwg02-canboatjs' ||
value.options.type === 'ydwg02' ||
value.options.type === 'navlink2') && (
<div>
<HostInput value={value.options} onChange={onChange} />
<PortInput value={value.options} onChange={onChange} />
Expand All @@ -1741,9 +1771,14 @@ function NMEA2000({ value, onChange, hasAnalyzer }: TypeComponentProps) {
/>
</div>
)}
{value.options.type === 'ydwg02-udp-canboatjs' && (
{(value.options.type === 'ydwg02-udp-canboatjs' ||
value.options.type === 'ydwg02-udp') && (
<div>
<HostInput value={value.options} onChange={onChange} />
{/* The native bridge listens on a UDP port and never dials a
host, so a Host value there would be saved and ignored. */}
{value.options.type !== 'ydwg02-udp' && (
<HostInput value={value.options} onChange={onChange} />
)}
<PortInput value={value.options} onChange={onChange} />
Comment thread
coderabbitai[bot] marked this conversation as resolved.
<div className="text-muted small mt-1 mb-2">
UDP is receive-only — N2K device discovery and PGN 126208 instance
Expand Down Expand Up @@ -1807,7 +1842,8 @@ function NMEA2000({ value, onChange, hasAnalyzer }: TypeComponentProps) {
<CollectNetworkStatsInput value={value.options} onChange={onChange} />
)}
{(value.options.type === 'w2k-1-n2k-ascii-canboatjs' ||
value.options.type === 'w2k-1-n2k-actisense-canboatjs') && (
value.options.type === 'w2k-1-n2k-actisense-canboatjs' ||
value.options.type === 'w2k-1-ascii') && (
<div>
<HostInput value={value.options} onChange={onChange} />
<PortInput value={value.options} onChange={onChange} />
Expand Down Expand Up @@ -1852,7 +1888,8 @@ function NMEA2000({ value, onChange, hasAnalyzer }: TypeComponentProps) {
</div>
</div>
)}
{value.options.type === 'maretron-ipg-canboatjs' && (
{(value.options.type === 'maretron-ipg-canboatjs' ||
value.options.type === 'maretron-ipg') && (
Comment thread
coderabbitai[bot] marked this conversation as resolved.
<div>
<HostInput value={value.options} onChange={onChange} />
<PortInput value={value.options} onChange={onChange} />
Expand All @@ -1876,7 +1913,9 @@ function NMEA2000({ value, onChange, hasAnalyzer }: TypeComponentProps) {
</Form.Group>
<div className="text-muted small mt-1 mb-2">
Maretron IPG 100 Ethernet gateway. Default TCP port 6543. Uses
Maretron&apos;s 0xA5-framed binary protocol (handled by canboatjs).
Maretron&apos;s 0xA5-framed binary protocol. The password is sent in
the clear during the gateway&apos;s own connection handshake — the
protocol offers no encrypted alternative.
</div>
</div>
)}
Expand Down
11 changes: 10 additions & 1 deletion packages/streams/src/execute.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ interface ExecuteOptions {
setProviderError(id: string, msg: string): void
}
providerId: string
toChildProcess?: string
toChildProcess?: string | false
restartOnClose?: boolean
restartThrottleTime?: number
createDebug?: CreateDebug
Expand Down Expand Up @@ -50,6 +50,15 @@ export default class Execute extends Transform {
this.pipeTo = pipeTo as unknown as Writable
this.startProcess(this.options.command)

// `false` disables stdin wiring entirely, for gateways that cannot
// transmit (the YDWG-02's receive-only UDP mode). Leaving the
// option undefined subscribes to the default event instead, so
// absence alone would still write to the child's stdin.
if (this.options.toChildProcess === false) {
this.debug('Receive-only provider: not wiring the child process stdin')
return super.pipe(pipeTo)
}

const stdOutEvent = this.options.toChildProcess ?? 'toChildProcess'
this.debug(
'Using event ' + stdOutEvent + " for output to child process's stdin"
Expand Down
75 changes: 71 additions & 4 deletions packages/streams/src/simple.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { Transform, TransformCallback } from 'stream'
import shellescape from 'any-shell-escape'
import N2kAnalyzer from './n2kAnalyzer'
import FromJson from './from_json'
import MultiplexedLog from './multiplexedlog'
Expand Down Expand Up @@ -396,14 +397,80 @@ function nmea2000input(
})
]
} else {
// Native (canboat) sources: each command bridges the gateway to canboat
// PLAIN/FAST CSV on stdout, which the downstream N2kAnalyzer decodes.
// Where toChildProcess is set, the same CSV format is accepted on stdin
// for outbound PGNs (encoded by canboatjs in execute.ts).
//
// Execute runs the command through a shell, so every provider-config
// value is validated before interpolation: hosts/devices/interfaces
// against the character set their legitimate values need, numbers as
// finite. Anything else fails the provider loudly.
const safeArg = (value: unknown, what: string): string => {
const s = String(value ?? '')
if (!/^[A-Za-z0-9_.:/-]+$/.test(s)) {
throw new Error(`invalid ${what} for NMEA 2000 connection: '${s}'`)
}
return s
}
const safeNum = (value: unknown, what: string): number => {
const n = Number(value)
if (!Number.isFinite(n)) {
throw new Error(
`invalid ${what} for NMEA 2000 connection: '${String(value)}'`
)
}
return n
}
let command: string
let toChildProcess: string | undefined
let toChildProcess: string | false | undefined
if (subOptions.type === 'ngt-1') {
command = `actisense-serial -s ${subOptions.baudrate ?? 115200} ${subOptions.device}`
command = `actisense-serial -s ${safeNum(subOptions.baudrate ?? 115200, 'baud rate')} ${safeArg(subOptions.device, 'device')}`
toChildProcess = 'nmea2000out'
} else if (subOptions.type === 'canbus') {
command = `candump ${subOptions.interface} | candump2analyzer`
toChildProcess = undefined
// canboat's socketcan-serial rather than candump|candump2analyzer:
// same decode path, but bidirectional — it claims an address and
// accepts outbound PGNs on stdin, which the candump pipeline cannot.
// -u/-m mirror the uniqueNumber/mfgCode settings the canboatjs
// SocketCAN option honors.
// Absence, not falsiness: 0 is a valid manufacturer code (the
// form offers it as "Internal") and a valid unique number.
const hasValue = (v: unknown) => v !== undefined && v !== null && v !== ''
command =
`socketcan-serial` +
(hasValue(subOptions.uniqueNumber)
? ` -u ${safeNum(subOptions.uniqueNumber, 'unique number')}`
: '') +
(hasValue(subOptions.mfgCode)
? ` -m ${safeNum(subOptions.mfgCode, 'manufacturer code')}`
: '') +
` ${safeArg(subOptions.interface, 'interface')}`
toChildProcess = 'nmea2000out'
} else if (subOptions.type === 'ikonvert') {
command = `ikonvert-serial -s ${safeNum(subOptions.baudrate ?? 230400, 'baud rate')} ${safeArg(subOptions.device, 'device')}`
toChildProcess = 'nmea2000out'
} else if (subOptions.type === 'maretron-ipg') {
const password = (subOptions as { password?: string }).password
command =
`maretron-ipg` +
(password ? ` --password=${shellescape(password)}` : '') +
` tcp://${safeArg(subOptions.host, 'host')}:${safeNum(subOptions.port ?? 6543, 'port')}`
toChildProcess = 'nmea2000out'
Comment thread
coderabbitai[bot] marked this conversation as resolved.
} else if (subOptions.type === 'ydwg02') {
command = `canboat interface --kind ydwg tcp://${safeArg(subOptions.host, 'host')}:${safeNum(subOptions.port ?? 1457, 'port')}`
toChildProcess = 'nmea2000out'
} else if (subOptions.type === 'ydwg02-udp') {
// The YDWG-02's UDP mode is receive-only: `false` disables the
// child's stdin wiring outright, so no plugin's outbound PGN can
// reach a gateway that cannot transmit.
command = `canboat interface --kind ydwg udp://${safeNum(subOptions.port ?? 1457, 'port')}`
Comment thread
coderabbitai[bot] marked this conversation as resolved.
toChildProcess = false
} else if (subOptions.type === 'navlink2') {
command = `canboat interface --kind ikonvert tcp://${safeArg(subOptions.host, 'host')}:${safeNum(subOptions.port ?? 6001, 'port')}`
toChildProcess = 'nmea2000out'
} else if (subOptions.type === 'w2k-1-ascii') {
command = `canboat interface --kind w2k-ascii tcp://${safeArg(subOptions.host, 'host')}:${safeNum(subOptions.port ?? 60002, 'port')}`
toChildProcess = 'nmea2000out'
} else {
throw new Error(`unknown NMEA2000 type ${subOptions.type}`)
}
Expand Down
Loading