kmbox net binary

[ramseymcgrath]

No description provided.

[roomote-v0]

Rooviewer   See task

Re-reviewed changes since 2423ffc. The .gitignore was updated to exclude the binary, but it's still tracked in git. The issue remains unresolved:

• Remove committed binary tools/kmbox_relay from git and add to .gitignore

Previous reviews

• 2423ffc: Review #1

_{Mention @roomote in a comment to request specific changes to this pull request or fix all unresolved issues.}

[Copilot]

Pull request overview

This pull request introduces a new KMBox Net UDP relay tool and enhances the humanization system with improved noise modeling. The PR titled "kmbox net binary" adds protocol bridge functionality and refines the output-stage humanization for more natural mouse movement simulation.

Changes:

• New kmbox_relay tool that translates KMBox Net UDP protocol to Wire Protocol v2 over serial, enabling compatibility with existing KMBox client applications
• Enhanced humanization with stochastic rounding, Gaussian-based sensor noise, and timing jitter to better simulate real optical mouse behavior
• Watchdog simplification by removing verbose startup delays and improving parameter passing
• Code cleanup removing unused functions, deprecated constants, and improving formatting consistency

Reviewed changes

Copilot reviewed 12 out of 13 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
tools/kmbox_relay.c	New 988-line UDP relay implementing KMBox Net protocol bridge with XXTEA encryption, cross-platform socket/serial support, and Wire Protocol v2 translation
tools/kmbox_relay	macOS binary executable (should not be committed to repository)
tools/Makefile	Updated build targets for kmbox_relay with Linux/macOS/Windows support
usb_hid.c	Added output-stage PRNG, stochastic rounding for sub-pixel tremor, Gaussian sensor noise, and timing jitter for humanization
watchdog.c	Simplified startup by removing extended delays, improved parameter passing to avoid redundant time_ms calls
state_management.h/c	Removed unused helper functions (system_state_should_run_task, system_state_batch_update_timers)
defines.h	Consolidated logging flags, removed deprecated humanization color constants
PIOKMbox.c	Fixed indentation in button handling code, removed unused structures and reporting functions
bridge/CMakeLists.txt	Improved PICO_SDK_PATH resolution with proper precedence order
build.sh	Added automatic git submodule initialization check
README.md	Updated clone instructions to include --recursive flag for submodules
.gitmodules	Added Pico-PIO-USB as git submodule

Comments suppressed due to low confidence (5)

usb_hid.c:1776

• The static sub-pixel accumulators (subpx_accum_x and subpx_accum_y) could theoretically grow unbounded if noise_gate is very small for extended periods, causing noise accumulation without triggering the ±1 thresholds. Consider adding periodic decay or clamping to prevent long-term drift, especially if the device runs for extended periods with minimal movement.

            static float subpx_accum_x = 0.0f;
            static float subpx_accum_y = 0.0f;
            float subpx_noise = 0.45f * noise_gate;
            subpx_accum_x += hid_rng_gaussian() * subpx_noise;
            subpx_accum_y += hid_rng_gaussian() * subpx_noise;
            if (subpx_accum_x >= 1.0f) { x += 1; subpx_accum_x -= 1.0f; }
            else if (subpx_accum_x <= -1.0f) { x -= 1; subpx_accum_x += 1.0f; }
            if (subpx_accum_y >= 1.0f) { y += 1; subpx_accum_y -= 1.0f; }
            else if (subpx_accum_y <= -1.0f) { y -= 1; subpx_accum_y += 1.0f; }

usb_hid.c:303

• The PRNG seed initialization uses 0xDEADBEEF as fallback when seed is 0. However, xorshift32 has a known issue where if the state is 0, it will produce an infinite sequence of zeros. While the current code prevents initial state = 0, if get_rand_32() returns 0 during initialization, the fallback handles it. This is correct but worth documenting the rationale in a comment for future maintainers.

static void hid_rng_seed(uint32_t seed) {
    hid_rng_state = seed ? seed : 0xDEADBEEF;
}

tools/Makefile:14

• The kmbox_relay build rule on line 14 doesn't explicitly link required libraries. On Linux systems, this may need -lpthread or other libraries depending on the platform. Consider adding platform-specific library flags or testing on Linux to ensure it builds correctly. The Windows cross-compile rule correctly includes -lws2_32.

kmbox_relay: kmbox_relay.c ../lib/wire-protocol/include/wire_protocol.h
	$(CC) $(CFLAGS) $(INCLUDES) -o $@ $<

tools/kmbox_relay.c:666

• The payload length calculation at line 666 could result in a negative value if len < KMNET_HEAD_SIZE for non-encrypted packets. While line 621 checks this case, the check uses !encrypted which may not trigger for all cases where len < KMNET_HEAD_SIZE. Consider adding explicit bounds checking before computing payload_len to ensure it's never negative, or reordering the checks to be more defensive.

    /* Extract payloads */
    const uint8_t *payload = buf + KMNET_HEAD_SIZE;
    int payload_len = (encrypted ? KMNET_ENC_SIZE : len) - (int)KMNET_HEAD_SIZE;

tools/kmbox_relay.c:472

• The encryption key derivation uses only 4 bytes from the MAC address (lines 468-471), resulting in very weak encryption (32-bit keyspace). While this matches the original KMBox Net protocol for compatibility, it provides minimal security. Consider documenting this security limitation in the file header comments, especially since the relay accepts connections over UDP which could be exposed to untrusted networks. Users should be aware that the encryption provides obfuscation but not true security.

static void derive_key(relay_t *r, uint32_t mac) {
    memset(r->enc_key, 0, 16);
    r->enc_key[0] = (mac >> 24) & 0xFF;
    r->enc_key[1] = (mac >> 16) & 0xFF;
    r->enc_key[2] = (mac >> 8)  & 0xFF;
    r->enc_key[3] = (mac >> 0)  & 0xFF;
}

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[roomote-v0]

The compiled binary tools/kmbox_relay was committed to the repository. This binary should be removed and added to .gitignore (similar to how tools/mouse_counteract is already excluded on line 16 of .gitignore). Committing binaries bloats the repository and makes it harder to track meaningful changes.

_{Fix it with Roo Code or mention @roomote and request a fix.}