Conversation
- Ensures pixel shift uses rounded and parsed number values for accurate shifts. - Modifies MQTT connection to ensure the clientId is properly set. - Introduces utilities for transforming unknown values to boolean and number types. - Adds `pins` data to the board connection state.
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
There was a problem hiding this comment.
Pull request overview
This PR improves pixel shift accuracy and MQTT connection reliability. The changes ensure pixel shifts use properly parsed and rounded numeric values, fix MQTT client ID handling by using object-based connection options instead of URL strings, and enhance type transformation utilities with better structure using switch statements.
Key changes:
- Adds
transformValueToNumberto pixel shift logic with rounding for accuracy - Refactors MQTT connection to use object-based options ensuring proper clientId configuration
- Adds
pinsdata tracking to board connection state for better state management
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
packages/runtime/src/pixel/pixel.ts |
Adds number transformation and rounding to forward pixel shifts; removes verbose console warning |
packages/runtime/src/_utils/transformUnknownValues.ts |
Refactors boolean and number transformation utilities using switch statements for clearer type handling |
packages/mqtt-provider/src/stores/mqtt.ts |
Changes MQTT connection from URL string to object-based options for proper clientId handling; adds path normalization |
apps/electron-app/src/main/board-connection.ts |
Introduces currentPins tracking variable and adds it to connection state responses |
Comments suppressed due to low confidence (3)
apps/electron-app/src/main/board-connection.ts:8
- Unused import BOARDS.
import {
BOARDS,
Flasher,
getConnectedPorts,
UnableToOpenSerialConnection,
type BoardName,
type PortInfo,
} from '@microflow/flasher';
packages/mqtt-provider/src/stores/mqtt.ts:225
- Unused variable callback.
for (const [topic, { callback, options }] of Array.from(subscriptions)) {
packages/mqtt-provider/src/stores/mqtt.ts:225
- For loop variable callback is not used in the loop body.
for (const [topic, { callback, options }] of Array.from(subscriptions)) {
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const connectionOptions: mqtt.IClientOptions = { | ||
| username: config.username, | ||
| password: config.password, | ||
| protocol: protocol === 'wss' ? 'wss' : protocol === 'ws' ? 'ws' : 'wss', |
There was a problem hiding this comment.
The protocol variable is already typed as 'ws' | 'wss' from parseMqttUrl, so this ternary expression is redundant. You can simplify this to just protocol: protocol or use the shorthand protocol.
| protocol: protocol === 'wss' ? 'wss' : protocol === 'ws' ? 'ws' : 'wss', | |
| protocol, |
| // reconnectPeriod: 1000, // Reconnect after 1 second | ||
| // For WSS connections, ensure proper SSL handling | ||
| ...(protocol === 'wss' | ||
| ...(protocol === 'wss' || protocol === 'ws' |
There was a problem hiding this comment.
SSL options like rejectUnauthorized should only be applied to secure WebSocket connections ('wss'), not to regular WebSocket connections ('ws'). The condition should be protocol === 'wss' only, not include 'ws'.
| ...(protocol === 'wss' || protocol === 'ws' | |
| ...(protocol === 'wss' |
| case 'number': | ||
| return value > 0; | ||
| case 'string': | ||
| const isTruthy = ['1', 'true', 'on', 'yes'].includes(String(value).toLowerCase()); |
There was a problem hiding this comment.
The value is already confirmed to be a string by the switch statement, so using String(value) is redundant. Consider using value.toLowerCase() directly.
| const isTruthy = ['1', 'true', 'on', 'yes'].includes(String(value).toLowerCase()); | |
| const isTruthy = ['1', 'true', 'on', 'yes'].includes(value.toLowerCase()); |
pinsdata to the board connection state.