Skip to content

fix: use discrete palette value steps #86

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
zhouxiyu1997 merged 1 commit into from
Jul 27, 2026
Merged

fix: use discrete palette value steps #86

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
20 changes: 1 addition & 19 deletions apps/desktop/src/protocol/paletteTiming.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ const COLOR_PALETTE_EDITOR_RESET_UP_HOLD_MS = 1_500;
const COLOR_PALETTE_EDITOR_RESET_LEFT_HOLD_MS = 3_000;
const COLOR_PALETTE_EDITOR_MOVE_STEP_MS = 20;
const COLOR_PALETTE_EDITOR_HUE_RESET_SETTLE_MS = 500;
const COLOR_PALETTE_EDITOR_DARK_VALUE_FINE_STEPS = 12;
const BASIC_COLOR_GRID_ROWS = 7;
const BASIC_COLOR_GRID_COLS = 12;
const BASIC_COLOR_TAB_SETTLE_MS = 140;
Expand Down Expand Up @@ -113,19 +112,6 @@ function rgbToHsv(red: number, green: number, blue: number): HsvColor {
};
}

function splitPaletteValueDropSteps(valueDropSteps: number): {
coarseValueSteps: number;
fineValueSteps: number;
} {
const normalizedSteps = valueDropSteps < 0 ? 0 : valueDropSteps;
const fineValueSteps = Math.min(normalizedSteps, COLOR_PALETTE_EDITOR_DARK_VALUE_FINE_STEPS);

return {
coarseValueSteps: normalizedSteps - fineValueSteps,
fineValueSteps,
};
}

function timeoutMargin(options: PaletteTimingOptions): number {
return options.includeTimeoutMargin ? PALETTE_CONFIG_TIMEOUT_MARGIN_MS : 0;
}
Expand Down Expand Up @@ -195,7 +181,6 @@ export function estimatePaletteConfigDurationMs(
const hueSteps = Math.round(hueRatio * COLOR_PALETTE_EDITOR_HUE_STEP_COUNT);
const saturationSteps = scaleChannelToSteps(hsv.saturation, COLOR_PALETTE_EDITOR_SATURATION_STEP_COUNT);
const valueDropSteps = scaleChannelToSteps(1 - hsv.value, COLOR_PALETTE_EDITOR_VALUE_STEP_COUNT);
const { coarseValueSteps, fineValueSteps } = splitPaletteValueDropSteps(valueDropSteps);
const generalPressMs = timing.buttonPressMs + timing.inputDelayMs;
const menuPressMs = COLOR_PALETTE_MENU_PRESS_DURATION_MS + COLOR_PALETTE_MENU_INPUT_DELAY_MS;

Expand All @@ -213,10 +198,7 @@ export function estimatePaletteConfigDurationMs(
(saturationSteps > 0
? saturationSteps * COLOR_PALETTE_EDITOR_MOVE_STEP_MS + timing.inputDelayMs
: 0) +
(coarseValueSteps > 0
? coarseValueSteps * COLOR_PALETTE_EDITOR_MOVE_STEP_MS + timing.inputDelayMs
: 0) +
fineValueSteps * generalPressMs +
valueDropSteps * generalPressMs +
3 * menuPressMs +
timing.inputDelayMs +
timeoutMargin(options)
Expand Down
8 changes: 8 additions & 0 deletions apps/desktop/test/firmware-flash.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -287,9 +287,17 @@ test("controller firmware routes palette menu navigation through reliable input"
new URL("../src/serial/sender.ts", import.meta.url),
"utf8",
);
const paletteBody = /bool SwitchController::configurePaletteSlot[\s\S]*?(?=bool SwitchController::configureBasicPaletteSlot)/u
.exec(controllerSource)?.[0] ?? "";

assert.match(configSource, /COLOR_PALETTE_MENU_INPUT_DELAY_MS = 150/u);
assert.match(senderSource, /from "\.\.\/protocol\/paletteTiming\.js"/u);
assert.doesNotMatch(paletteBody, /coarseValueSteps/u);
assert.doesNotMatch(paletteBody, /moveDirection\(\s*0,\s*1,/u);
assert.match(
paletteBody,
/for \(uint16_t step = 0; step < valueDropSteps; step \+= 1\)[\s\S]*ControllerButton::DpadDown/u,
);
assert.match(
transportHeaderSource,
/virtual bool pressButtonsReliable[\s\S]*return pressButtons\(buttonsMask, holdMs, settleMs\);[\s\S]*pressButtonReliable/u,
Expand Down
7 changes: 7 additions & 0 deletions apps/desktop/test/three-layer-fix.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { pixelizeImage } from "../src/image/pixelize.js";
import { renderPreviewToBuffer } from "../src/image/renderPreview.js";
import { resizeImage } from "../src/image/resizeImage.js";
import { generateScanlineCommands } from "../src/path/scanline.js";
import { estimatePaletteConfigDurationMs } from "../src/protocol/paletteTiming.js";
import { serializeCommands } from "../src/protocol/serializer.js";
import {
SERIAL_OPEN_BOOT_TIMEOUT_MS,
Expand Down Expand Up @@ -482,6 +483,10 @@ test("serial sender probes fresh ESP32 serial sessions before first sequenced co
test("palette-config commands get enough timeout for calibrated custom colors", () => {
const timing = { buttonPressMs: 100, inputDelayMs: 100, homeMs: 1800 };

assert.equal(estimatePaletteConfigDurationMs(1, 0x4e, 0x32, 0x39, timing), 34_660);
assert.equal(estimatePaletteConfigDurationMs(2, 0x00, 0xff, 0x00, timing), 46_560);
assert.equal(estimatePaletteConfigDurationMs(0, 0x20, 0x20, 0x20, timing), 35_680);
assert.equal(estimatePaletteConfigDurationMs(0, 0x00, 0x00, 0x00, timing), 38_480);
assert.equal(getAckTimeoutForCommand("PC 1 #4E3239", 20_000, timing), 90_000);
assert.equal(getAckTimeoutForCommand("PC 2 #00FF00", 20_000, timing), 90_000);
assert.equal(getAckTimeoutForCommand("PC 0 #202020", 20_000, timing), 90_000);
Expand All @@ -491,6 +496,8 @@ test("palette-config commands get enough timeout for calibrated custom colors",
test("palette-config timeout covers issue 74 custom colors", () => {
const timing = { buttonPressMs: 65, inputDelayMs: 45, homeMs: 1800 };

assert.equal(estimatePaletteConfigDurationMs(2, 0x93, 0x36, 0x43, timing), 23_845);
assert.equal(estimatePaletteConfigDurationMs(3, 0x5b, 0x2a, 0x33, timing), 26_065);
assert.equal(getAckTimeoutForCommand("PC 2 #933643", 20_000, timing), 90_000);
assert.equal(getAckTimeoutForCommand("PC 3 #5b2a33", 20_000, timing), 90_000);
});
Expand Down
1 change: 0 additions & 1 deletion firmware/esp32/src/config.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ constexpr uint16_t COLOR_PALETTE_EDITOR_RESET_UP_HOLD_MS = 1500;
constexpr uint16_t COLOR_PALETTE_EDITOR_RESET_LEFT_HOLD_MS = 3000;
constexpr uint16_t COLOR_PALETTE_EDITOR_MOVE_STEP_MS = 20;
constexpr uint16_t COLOR_PALETTE_EDITOR_HUE_RESET_SETTLE_MS = 500;
constexpr uint8_t COLOR_PALETTE_EDITOR_DARK_VALUE_FINE_STEPS = 12;
constexpr uint8_t BASIC_COLOR_GRID_ROWS = 7;
constexpr uint8_t BASIC_COLOR_GRID_COLS = 12;
constexpr uint16_t BASIC_COLOR_TAB_SETTLE_MS = 140;
Expand Down
15 changes: 1 addition & 14 deletions firmware/esp32/src/controller.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -293,10 +293,6 @@ bool SwitchController::configurePaletteSlot(int index, uint8_t red, uint8_t gree
scaleChannelToSteps(hsv.saturation, COLOR_PALETTE_EDITOR_SATURATION_STEP_COUNT);
const uint8_t valueDropSteps =
scaleChannelToSteps(1.0f - hsv.value, COLOR_PALETTE_EDITOR_VALUE_STEP_COUNT);
const uint8_t fineValueSteps = valueDropSteps < COLOR_PALETTE_EDITOR_DARK_VALUE_FINE_STEPS
? valueDropSteps
: COLOR_PALETTE_EDITOR_DARK_VALUE_FINE_STEPS;
const uint8_t coarseValueSteps = valueDropSteps - fineValueSteps;

// Palette selection page.
if (!pressPaletteMenuButton(transport_, ControllerButton::Y)) {
Expand Down Expand Up @@ -359,16 +355,7 @@ bool SwitchController::configurePaletteSlot(int index, uint8_t red, uint8_t gree
}
}

if (coarseValueSteps > 0) {
if (!transport_.moveDirection(
0, 1, static_cast<uint16_t>(coarseValueSteps) * COLOR_PALETTE_EDITOR_MOVE_STEP_MS, inputDelayMs_)) {
return false;
}
}

// Keep the last darkening steps discrete so near-black shades do not get
// flattened by the continuous analog hold.
for (uint16_t step = 0; step < fineValueSteps; step += 1) {
for (uint16_t step = 0; step < valueDropSteps; step += 1) {
if (!transport_.pressButton(ControllerButton::DpadDown, buttonPressMs_, inputDelayMs_)) {
return false;
}
Expand Down
Loading