Skip to content

Bits and bobs from the SC2 fork - #116

Open
C0rn3j wants to merge 111 commits into
mainfrom
sc2
Open

Bits and bobs from the SC2 fork#116
C0rn3j wants to merge 111 commits into
mainfrom
sc2

Conversation

@C0rn3j

@C0rn3j C0rn3j commented Aug 2, 2026

Copy link
Copy Markdown
Owner

Rather reworked version of #100 and its follow-up hard fork, kept rebased against main.

This is mostly for myself, to have an easy shortcut to view what's left to check out, to either implement it or not.

@C0rn3j
C0rn3j force-pushed the sc2 branch 3 times, most recently from f4dae15 to ce8d822 Compare August 2, 2026 21:11
@C0rn3j
C0rn3j force-pushed the main branch 2 times, most recently from 69fd146 to 2178249 Compare August 9, 2026 14:38
@C0rn3j
C0rn3j force-pushed the sc2 branch 2 times, most recently from b4bff7e to d2a6658 Compare August 13, 2026 17:11
@C0rn3j
C0rn3j force-pushed the sc2 branch 7 times, most recently from 459b960 to 433feeb Compare August 17, 2026 20:04
@C0rn3j
C0rn3j force-pushed the sc2 branch 3 times, most recently from d99c6d7 to 0dc7ace Compare August 19, 2026 16:08
C0rn3j and others added 12 commits August 19, 2026 18:41
…ness

Reverse-engineered the new Steam Controller's main gamepad HID report
(0x42) from live captures of real hardware via its wireless Puck
(28de:1304). Documents the full byte layout: 4-byte button bitfield
(incl. capacitive stick/pad/grip touch and analog+digital triggers),
two analog sticks, two trackpads with pressure, and 16-bit triggers.

Notes that the IMU is disabled by default and the controller defaults to
lizard mode; both the command channel (lizard-off, gyro-on) and the IMU
stream remain to be reverse-engineered.

Adds tools/sc2-probe/, the read-only hidraw capture harness used to
produce these findings.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

docs: add v2 command channel (from Steam usbmon capture)

Sniffed Steam's USB traffic while it configured the controller and
decoded the host->device command protocol: SET_REPORT (0x21/0x09) with
wValue=0x03<id> (feature) / 0x02<id> (output), wIndex=interface (per
slot), 64-byte [reportID, packetType, length, params] payloads.

Opcodes match sc_dongle.py's SCPacketType: 0x81 CLEAR_MAPPINGS (lizard
disable, resent as heartbeat), 0x8E LIZARD_MODE, 0x87 CONFIGURE/LED,
0xAE GET_SERIAL, 0xC1 SET_AUDIO_INDICES, plus v2-only key/value config
(0xED "user/wireless_transport", "esb/bond"). LED level confirmed as
87 03 2d <level>. Gyro-enable register still TBD.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

docs: confirm gyro enable command and IMU location

Live experiment (rotate controller, toggle gyro): the byte after `87 0f 30`
is the gyro/accel enable -- 0x18 on, 0x00 off -- and once enabled the IMU
streams in report 0x42 at offsets ~31-53 (bytes 31-53 go from static to
60-256 distinct values when moving). Matches what Steam sends. The driver's
configure() already emits 0x18; parse_input still zeroes the gyro fields
pending decode of the accel/gyro/quaternion sub-layout.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

sc2: scaffold the new Steam Controller (v2) driver

New scc/drivers/sc2.py implementing the reverse-engineered v2 protocol:
report 0x42 parsing (buttons, two sticks, two pads with pressure, analog
+ digital triggers, d-pad, grips/paddles, capacitive touch), mapped to
SCButtons; the wireless Puck (0x1304) as a 4-slot dongle; and the v2
command transport (SET_REPORT to feature report 0x01 per interface) with
CLEAR_MAPPINGS unlizard heartbeat + replayed CONFIGURE/LED blocks.

Modeled on steamdeck.py (parsing/mapping) and sc_dongle.py (multi-slot +
commands). Gyro enable, haptics, real GET_SERIAL read-back, the wired
(0x1302)/Bluetooth (0x1303) transports, GUI assets and live testing are
still TODO (marked inline). tests/test_sc2.py locks the 0x42 layout with
synthetic frames (no hardware needed); 11 tests pass.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

sc2: lenient input transfer for mixed-length reports

Live bring-up validated the protocol (lizard-off via CLEAR_MAPPINGS,
buttons/sticks/triggers/pads all decode correctly on real hardware), but
exposed an integration bug: the puck's interrupt-IN endpoint multiplexes
reports of several sizes (0x42=54B, plus shorter 0x43/0x44/0x7b). The
shared USBDevice.set_input_interrupt drops and stops resubmitting any
report whose length != the requested size, which would freeze input on
the first short report. Replace it with a per-driver lenient transfer
that requests the full 64-byte max packet, accepts any length, filters by
report ID in parse_input, and always resubmits.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

sc2: enable driver + fix live bring-up bugs

End-to-end bring-up in scc-daemon on real hardware now works: the puck is
detected, the controller registers, lizard mode is disabled, and button /
stick / pad / trigger input reaches uinput (verified digital -> BTN_* and
analog -> ABS_X/Y).

Fixes found during bring-up:
- enable the driver by default (config.py "drivers": add "sc2": True);
  it was skipped as a disabled driver.
- SET_REPORT length: command builders no longer pre-pad to 64; send_control
  prepends the 0x01 report-ID byte and clamps to exactly 64 bytes. A 65-byte
  transfer was stalling the device (LIBUSB_ERROR_PIPE) on the first command.
- override disconnected() as a no-op (the inherited SCController version
  touches a dongle-only _available_serials attribute and crashed on unplug).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

sc2: decode and parse the IMU (accel / quaternion / gyro)

Captured isolated rotations with the gyro enabled and decoded report 0x42's
IMU block (offsets 30-53): 30-33 timestamp, 34-39 accelerometer (Z holds
~1g at rest), 40-47 orientation quaternion (w~32767 at rest), 48-53 gyro
pitch/roll/yaw. Verified each gyro axis dominates only its own motion
(pitch->@48, roll->@50, yaw->@52) and accel_z tracks gravity.

parse_input now fills accel_x/y/z, gpitch/groll/gyaw and q1..q4 from these
offsets instead of zeroing them; configure() already enables the gyro.
Accel X/Y labels and IMU signs remain provisional (polarity TBD). Adds IMU
assertions to the parser test (12 tests pass).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

sc2: map the 4th system button (View)

The controller has four system buttons, not three: the View button (⧉,
top-left) was untested and unmapped. Found at off3 bit 0x40 (it also emits
a lizard keyboard report). Mapped View -> BACK, and moved QuickAccess (…)
from BACK to DOTS so the four map cleanly to C / START / BACK / DOTS
(Steam / Menu / View / QuickAccess). off3 is now fully mapped.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

sc2: fix gyro pitch polarity (verified live)

Loaded a gyro->mouse profile in scc-daemon and checked cursor direction:
yaw is natural (right->right) but pitch was inverted (up->down). Negated
gpitch in parse_input so pitch-up aims up; re-verified live (up->up,
right->right). Gyro roll sign remains untested/provisional.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

sc2: implement click haptics (output report 0x82)

Captured Steam's trackpad haptic feedback and decoded the rumble command:
output report 0x82 = [0x82, side, effect, amplitude] on the interrupt-OUT
endpoint (number == interface). side 0/1/2 = left/right/both, effect 0x01
= click (0x02 longer), amplitude 0x00(medium)..0xff(strong). The device
stalls this report over SET_REPORT control, so feedback() submits an
interrupt-OUT transfer instead. Verified live via the daemon's Feedback
command: right/left/both clicks land on the correct side.

It's a per-call click (fits pad/scroll detents); continuous variable
rumble, if supported, would use a yet-uncaptured report.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

sc2: support the wired (USB-C, 0x1302) transport

The cabled controller enumerates as a single HID interface 0 (interrupt IN
0x81 / OUT 0x01, no CDC) with the same report descriptor and 0x42 report as
the puck, so everything reuses. Refactor the USB device into a shared
SC2Device base (lenient interrupt-IN, SET_REPORT/feature-0x01 commands,
interrupt-OUT haptics, controller bookkeeping) with SC2Puck (4 slots) and
SC2Wired (interface 0) subclasses, and give SC2Controller an explicit
out-endpoint (puck OUT ep == interface; wired OUT ep == 1). Register 0x1302.

Verified live over USB-C: detection, registration, buttons/sticks input,
and L/R/both haptics all work.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

sc2: GUI controller config (images/sc2.config.json)

get_gui_config_file() now returns "sc2.config.json" so the GUI renders the
controller with its real buttons/axes/gyro. The v2's controls match the
Steam Deck, so the config mirrors deck.config.json and reuses the "deck"
background image for now (a dedicated controller-images/sc2.svg is TODO).
Verified the daemon advertises it: "Controller: <id> sc2 19 sc2.config.json".

The core SC/Deck drivers have no GUI enable/disable toggle (always on), so
sc2 follows suit -- no global_settings change needed.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The modeshift "combination" list only had Left/Right Grip -- the original
SC's two back buttons. The Deck and the new Steam Controller have four back
buttons (L4/R4 -> LGRIP/RGRIP, L5/R5 -> LGRIP2/RGRIP2) and a right stick
(R3 -> RSTICKPRESS). Add LGRIP2, RGRIP2 and RSTICKPRESS to the chooser so
those can be used as modeshift combinations. Benefits the Deck too.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Same gap as the modeshift chooser: the d-pad-emulation source picker
(ae/dpad.glade), the special-action button picker (ae/special_action.glade)
and the controller-settings picker (controller_settings.glade) only listed
Left/Right Grip and Stick Press. Add Left/Right Grip 2 (LGRIP2/RGRIP2 = the
L5/R5 back buttons) and Right Stick Press (RSTICKPRESS) so every binding
dialog offers the full Deck / new-Steam-Controller button set.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Two gaps the original SC's button set didn't cover:

- The capacitive stick-touch sensors had no SCButtons constants, so the
  decoded bits (LStick = off5 0x01, RStick = off4 0x10) were unmapped (the
  Deck leaves them out for the same reason). Add SCButtons.LSTICKTOUCH /
  RSTICKTOUCH (free bits 16/17), map them in the v2 driver, and add
  "Left/Right Stick Touched" to all four button choosers (modeshift +
  ae/dpad, ae/special_action, controller_settings).
- Now that there's a "Right Stick Pressed", relabel the old "Stick
  Pressed" / "Stick Press" to "Left Stick Pressed" / "Left Stick Press".

Driver mapping unit-tested; full suite (157) passes. (The Steam Deck driver
could now map its stick-touch bits too, via the same constants.)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The Deck reports stick-touch (DeckButton.LSTICKTOUCH/RSTICKTOUCH) but they
were left unmapped because SCButtons had no equivalent. Now that
SCButtons.LSTICKTOUCH/RSTICKTOUCH exist (added for the new controller), map
the Deck's bits too, so "Left/Right Stick Touched" works on the Deck as
well. The shared mapper/action and GUI-chooser fixes already cover the Deck.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The new Steam Controller (like the v1) has capacitive sensors on the
handles -- distinct from the L4/L5/R4/R5 grip buttons -- which the Steam
Deck lacks. Decoded as off5 0x20 (left) / 0x10 (right). Add
SCButtons.LGRIPTOUCH/RGRIPTOUCH (free bits 18/19), map them in the v2
driver, and add "Left/Right Grip Sensing" to all four button choosers
(modeshift + ae/dpad, ae/special_action, controller_settings). These read
"on" whenever the handles are held, which suits grip-activated modeshifts.

Driver mapping unit-tested; full suite (158) passes.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

gui: rename grip-sensing labels to "Grip Touched"

Match the thumbstick-sensor labels ("Left/Right Stick Touched"): the
capacitive handle grips are now "Left/Right Grip Touched" in all four
button choosers. Label only; the LGRIPTOUCH/RGRIPTOUCH constants are
unchanged.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…nsor bindings

Replace the borrowed Steam Deck GUI image with dedicated v2 artwork and add
first-class support for the controller's capacitive sensors.

Controller image & assets (generated by tools/gen_sc2_image.py from
tools/sc2-source.svg + tools/sc2-assets/):
- controller-images/sc2.svg: traced v2 body, blank face buttons, control-name
  ids so sticks/pads/dpad/bumpers/grips highlight on hover, darker body.
- button-images/sc2_*.svg: v2 face-button overlay glyphs lifted from the art
  (monochrome ABXY, round Steam, single dots, view/menu) - no duplication.
- images/sc2/*.svg: v2-specific side-panel icons (leaned-square pads, real
  view/menu, oval L4/R4/L5/R5 paddles, grip-touch silhouettes).
- sc2.config.json points at all of the above.

Capacitive sensors:
- Stick-touch: new "Touch" tab in the stick's pressed-action editor
  (ModeshiftEditor) binds LSTICKTOUCH/RSTICKTOUCH; shown only for the stick
  press, hidden elsewhere.
- Grip-touch: exposed on the controller face (curved handle overlay, green on
  hover) and as buttons in the side-panel grid.
- Both usable as conditions in mode-shift combinations.

Fixes:
- Per-controller side-panel icon override (images/<background>/<name>.svg),
  leaving v1/Deck untouched.
- Right-stick (and center-pad) "pressed action" now opens the editor
  (RSTICK->RSTICKPRESS, CPAD->CPADPRESS).
- set_action no longer throws when saving a button with no on-screen widget
  (the touch sensors).

README: note v2 support + the stick-touch/grip-sensor binding & combinations.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

docs: correct Steam Controller 2 release year to 2026 in README

Matches the year correction already applied to the code comments/config.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
- Install a no-op Xlib error handler (xwrappers) so a stray X protocol error
  (e.g. a window that vanishes mid-query) no longer aborts the process.
- MenuData.generate now logs and skips a failing generator instead of letting
  it take down the whole menu.
… layout

"Display Current Bindings" always rendered the fixed v1 binding-display.svg
template and a hardcoded 5-box layout built for the v1 control set, so it showed
the v1 controller regardless of which one was connected, and its boxes overflowed
the screen on busier profiles.

- binding_display.py now resolves a per-controller image: an explicit
  gui.binding_display, else binding-display-<gui background>.svg (e.g.
  binding-display-sc2.svg), else the generic template. The window is built once
  the connected controller is known (on_daemon_connected) so it can pick the
  right image, and it draws that controller's current profile right away.

- The Generator box layout is per-controller now. The original 5-box layout is
  kept verbatim as the v1 fallback (_build_v1); a LAYOUTS table drives others.
  LAYOUTS["sc2"] is the Steam Deck-style v2 set: six boxes (system, left/right
  shoulder, left/right thumb, face) covering two sticks, a D-pad, two pads, four
  system buttons and the back paddles + grip-squeeze. Every control is listed but
  only bound ones draw a line, and a box with no bound controls is hidden - so
  grip-squeeze and the touch/press variants show up only when actually bound.

- Boxes auto-fit: a per-box max_height plus font auto-scaling shrinks a crowded
  box (e.g. a stick bound to a big radial) so all its lines stay inside it,
  fixing the overflow.

- tools/gen_binding_display.py generates images/binding-display-sc2.svg from the
  restyled controller art (tools/binding-display-sc2-art.svg) inlined verbatim,
  plus the AREA_* anchors of the GUI image, placing the six markers_<box>
  connector groups. Edit the art asset in Inkscape and re-run to regenerate.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…oller --osd)

The OSD menu's "Edit Bindings" runs `sc-controller --osd`, which used the
controller-driven "OSD mode" (osd_mode): it reused the full main window and drove
it by injecting X11-style GDK events and matching windows by XID. That only works
on the X11 backend, and even there it was fragile (a mispositioned, black-
rendering hint overlay); on Wayland it just spawned a duplicate main window.

--osd now opens only the standalone OSD-keyboard bindings editor instead - the
same dialog as Settings > Menus & Keyboard > Advanced - on both X11 and Wayland.
It is a plain GTK window with no backend dependency, so it behaves consistently
everywhere:

- no main window is shown (so it cannot pile up duplicate main windows) and no
  tray icon;
- the OSK.* actions are registered first so the OSD-keyboard profile parses;
- closing the editor quits the process;
- an flock-based single-instance guard makes a repeat launch a no-op instead of
  stacking a second editor window.

osd_mode is left in place but is now unreachable (osk_edit_mode replaces it); it
is removed in the next commit.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This is the single, isolated removal of osd_mode, kept separate from v0.4 so the
v0.4..v0.5 diff is the complete record of the feature should it ever be wanted
back.

What osd_mode was: launching `sc-controller --osd` opened the main window in a
special mode you navigated with the controller itself - the pad drove focus and
a floating hint overlay (OSDModeMappings) showed the button legend - so bindings
could be edited from the couch without a keyboard or mouse.

Why it is abandoned:
- X11 only. It drives the GUI by synthesising X11-style GDK input events
  (OSDModeKeyboard/OSDModeMouse via Gtk.main_do_event) and matches windows by
  XID. Under a native Wayland GDK backend none of that works: focus cannot move
  (GTK_IS_WIDGET warnings) and there is no XID to match.
- Even on X11 it is fragile: the hint overlay latches onto the wrong active
  window and renders black (it is an override-redirect window), and editing
  happens in the full main window rather than a focused dialog.
- As of v0.4 "Edit Bindings" (`sc-controller --osd`) opens the standalone
  OSD-keyboard bindings editor instead, on both X11 and Wayland - a plain GTK
  dialog that is consistent and reliable - which made osd_mode unreachable dead
  code (osk_edit_mode replaced it).

Removed: scc/gui/osd_mode.py (OSDModeMapper/Keyboard/Mouse/Mappings); App.osd_mode,
App.osd_mode_mapper and all their conditionals; App.enable_osd_mode and
OSD_MODE_PROF_NAME; the OsdmodeMappings window in glade/app.glade; the
on_Dialog_key_press_event handler and its glade signal (action_editor); the
osd_mode button-grab/name-entry guards (action_editor, ae/buttons); and the
now-unused default profile .scc-osd.profile_editor.sccprofile.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Patola and others added 29 commits August 19, 2026 18:41
scc.tools.find_library searches, in order: the repo root, its parent, then the
environment's site-packages. run.sh builds a wheel and installs it into .env,
so the copy it produces is the LAST candidate -- any lib*.so left in the repo
root by an old `build_ext --inplace` shadows it permanently.

One did. libuinput.cpython-314-x86_64-linux-gnu.so dated 2026-06-14 was being
loaded by every local run, while run.sh faithfully rebuilt the .env copy that
nothing opened. It cost most of a debugging session on the rumble work: the
new FF_RUMBLE magnitudes read back as zero while the averaged level next to
them was correct, which is exactly what a binary compiled against the older
20-byte struct does -- it writes level and never touches the two fields added
after it, leaving ctypes' zero-initialised bytes.

Every static check passed and kept pointing at correct code, because the code
WAS correct; the wrong binary was loaded. Clearing them costs nothing, since
run.sh rebuilds and installs on every invocation anyway.

Only scc/uinput.c has changed since those files were built, so the effect was
limited to this rumble work rather than any earlier driver testing.
Rumble confirmed live with fftest on the emulated pad: type, intensity and
both gains at 0, and the two speed fields driving their own actuator
independently, with Linux's strong/weak FF_RUMBLE magnitudes landing on left
and right as Valve's SDL3 driver arranges them.

Also records an open question the test raised. The field is called speed, not
amplitude, and gain is a separate i8 in dB. The weak-motor effect carries the
larger value (49152 against 32768) yet is clearly weaker in the hand, so speed
looks like a motor speed rather than a linear amplitude -- higher meaning a
lighter, higher-frequency buzz. Both gains are 0 dB only because that is what
SDL sends; per-side gain and the intensity field are untried and may be the
better volume control.
Refs #7. The synthesised effects from the v2's haptic family, output reports
0x83 (LFO tone), 0x84 (logarithmic sweep) and 0x85 (firmware script).

Two design decisions, both about not breaking what exists:

HapticData grows the effect and its parameters as defaulted keyword
arguments, kept OUT of the 'data' tuple. That tuple is unpacked positionally
by every existing driver, so growing it would break them; the new fields are
plain attributes that drivers read only if they understand them.

feedback() is left bit-for-bit compatible, because it appears in every
profile already written and its arguments are positional -- adding to it
would change the meaning of files on disk. The new effects are sibling
commands instead: feedbacktone, feedbacksweep and feedbackscript, sharing a
base class with feedback() so they attach to an action the same way.

Only hardware that can synthesise waveforms plays them, currently the v2
alone. Anything else ignores the effect field and plays its usual click, so
a profile written for one controller still does something on another rather
than going silent.

Report layouts verified against the documented sizes: tone 10 bytes, sweep 9,
script 4.

    TONE   -> 83 01 fe dc 00 2c 01 04 00 80
    SWEEP  -> 84 00 fe fa 00 90 01 50 00
    SCRIPT -> 85 01 03 fe

Also fixes HapticData.with_position, which passed self.frequency back to the
constructor even though it is already scaled by 1000 -- so it was multiplied
again on every copy, and send_feedback() copies for every HapticPos.BOTH
effect. And drops ControllerFlags from modifiers.py, unused since the pad/
stick predicate replaced its last reader.

The repo requires docs and both a parser and a profile test for every action;
all three are provided for each new command.
Refs #7. Front end for the tone/sweep/script effects added in 7086d91.

The Feedback panel gains an Effect chooser above the side selector, since it
decides what the rest of the panel means, and the parameter rows change with
it: tone gets frequency, duration and the two LFO controls; sweep gets start
and end frequency plus duration; preset gets a script id. Strength stays
throughout, and the two click-only sliders -- pad-travel frequency and click
period, neither of which means anything to a synthesised effect -- are hidden
unless Click is selected.

The chooser is hidden entirely on controllers that cannot synthesise
waveforms, rather than offering effects that would quietly degrade to a
click. Currently that means it appears for the v2 only.

Those rows are built in code rather than in the .glade. Six rows of
label+scale that are mostly hidden at any one time is a lot of XML to keep
correct by hand, and building them from a table keeps the rows, the
show/hide rule and the modifier arguments in step.

Each effect modifier now declares its own PARAMS, and the editor builds the
modifier positionally from that. Keeping the order in the GUI instead meant
two lists that had to agree, and they already did not: FeedbackScriptModifier
took (position, script_id, amplitude) while the editor passed amplitude
second, so choosing a preset would have written the strength into the script
id. Its signature now matches its siblings, and a test asserts PARAMS against
each _mod_init signature so the two cannot drift again.

That test deliberately imports only the modifiers. Importing the GUI from the
suite loads Gtk without a version request and breaks the version negotiation
in test_setup, which is how the first version of it failed.
The effect chooser was driven by a table in action_editor.py that paired each
HapticEffect with its modifier, and read .PARAMS off whichever it found. Only
the three new effect modifiers had that attribute; FeedbackModifier, which
carries plain Click, did not. Reading it raised during setup_widgets, so the
action editor failed to construct at all -- every binding, not just haptic
ones:

    AttributeError: type object 'FeedbackModifier' has no attribute 'PARAMS'

Give Click the same declarations as the others (it uses none of the extra
parameter rows, its knobs being feedback()'s own frequency and period, but it
still has to say so), and move the table to modifiers.py as
HAPTIC_EFFECT_MODIFIERS.

The move is the actual fix. The table lived in a module the test suite cannot
import -- importing the GUI loads Gtk without a version request and breaks
version negotiation in test_setup -- so nothing could check it, and a missing
attribute on one entry only showed up by opening the editor. In modifiers.py
it is plain data: two tests now walk every entry and assert each declares
EFFECT, PARAMS, LABEL and COMMAND with no duplicate effects, and that every
name in PARAMS is a field HapticData actually carries. Verified that removing
PARAMS again fails them.
Switching effects reset the parameters, and confirming the dialog came back
with feedback disabled -- sometimes losing unrelated settings such as the
mouse output. Preset lost them every time.

update_modifiers() is what decides an action is dirty: it diffs every widget
against state held on the editor and, on any difference, rebuilds by calling
set_action(). The effect chooser and its parameter rows were not in that
state. So changing them marked nothing dirty, and the next rebuild triggered
by anything else regenerated the action from state that had never heard of
them -- taking the effect, its parameters and whatever else was mid-edit.

Track feedback_effect and feedback_params alongside feedback_position, diff
them like everything else, and generate from the tracked values rather than
reading the widgets directly. Loading an action now restores the state as
well as the widgets, including resetting to Click, which a tone loaded before
a click action would otherwise have kept.

Adds tests/test_action_editor.py, which constructs the real editor against a
stub app and exercises this. The GUI had no tests because importing it loads
Gtk, and both bugs shipped this round would have been caught by opening the
editor once -- so the tests skip without a display and run under xvfb-run,
rather than not existing. They already earned it: they caught an off-by-one
reading the parameter defaults tuple.
The preset row was a bare 0-255 slider because I had assumed the script ids
were undocumented -- and then suggested probing "0 to 5", a number I had
carried over from an fftest effect list that has nothing to do with them.

They are documented: 16 named presets, 0x01 to 0x10, from the same reverse
engineering the rest of the haptic family came from. So the row is a dropdown
of names, and the ids are recorded in the protocol doc. Names are unverified
-- what each actually feels like is still to be checked on hardware -- and the
list may be partial, so an id we do not know is kept and shown as "Preset N"
rather than silently rewritten to something we do.

Parameter rows can now be either a dropdown or a slider, for parameters that
enumerate rather than scale, read and written through accessors so the rest
of the editor does not care which.

Also records in TODO why button presses still have no feedback option: it is
not an oversight, ButtonAction simply never grew set_haptic, and adding
MOD_FEEDBACK to it changes what the editor offers for buttons on every
controller -- so it wants its own pass.
Opening a saved binding brought up an editor with no action in it:
output "None", feedback switched off, side back to left, and only the
effect itself preserved.

load_modifiers() has two halves. The first parses the modifier chain
into editor state; the second writes that state into the widgets, and it
is guarded by _recursing because every widget write fires a 'changed'
handler that calls update_modifiers(). The effect chooser and its
parameter rows were being written in the FIRST half, unguarded, so:

  - the combo emits 'changed' -> update_modifiers() runs
  - it reads cbFeedback, still unchecked because that widget is not
    written until much further down, and concludes feedback is off
  - it marks the action dirty and calls set_action(self._action), which
    on a freshly opened editor is None

The editor then loads None and shows the first page, and the action
being edited never arrives. Which half of the panel survived told the
story: the effect was kept because it was the one thing already written.

Fixed by moving those widget writes down into the guarded block with
every other one. The parse loop now only records state.

Covered by test_opening_the_editor_keeps_the_feedback, which goes
through set_input() -- the entry point the dialog actually uses.
test_loading_an_effect_restores_the_widgets went the same way; it had
been setting _recursing by hand, and that fake is exactly what hid this.

Refs Patola#7

Co-Authored-By: Claude <noreply@anthropic.com>
A slider spanning 20-1000 Hz in about 200 pixels is roughly 5 Hz per
pixel, so there was no way to ask for 220 Hz specifically -- and the
arrow keys moved in jumps of 10, because the per-parameter step was
being used as the adjustment's step_increment.

Each numeric row is now a slider and a spin button sharing one
Gtk.Adjustment: drag to explore the range by feel, type or click the
spin arrows to land on a value exactly. Sharing the adjustment is what
keeps them honest -- there is no synchronising code that could get it
wrong, and the 'value-changed' handler hangs off the adjustment rather
than either widget. The slider no longer draws its own number, since the
spin button is already showing it.

Arrow keys and the spin button now always step by 1. The table's step
became the page increment instead, for Page Up/Down and trough clicks:
10 Hz for the frequencies, 50 ms for duration. Preset is untouched, it
is a named dropdown.

Wrapping each row in a Box made it reachable by show_all(), which would
have unhidden every parameter row at once, so hidden rows are marked
no-show-all. The new assertion in test_effect_rows_follow_the_chosen_effect
fails without it.

Refs Patola#7

Co-Authored-By: Claude <noreply@anthropic.com>
Reports 0x83, 0x84 and 0x85 are now verified live, not just read out of
the SDL3 source and iczero's RE:

  - a sweep is clearly distinguishable from a flat tone, so start_freq
    and end_freq do what they claim
  - the LFO fields work but are weak. At lfo_freq 4 / lfo_depth 128 the
    modulation is barely perceptible; only at 10 / 255 is it obvious, so
    depth is probably not a percentage
  - the preset ids are real and their RE'd names broadly describe what
    you feel. CONTROLLER_VERY_ON, CONTROLLER_OFF and PHONE_RINGING_1 are
    strong and unmistakable; several others are subtle
  - WILHELM_SCREAM is barely perceptible even at maximum gain. It is a
    long sample, so this is most likely the preset itself rather than
    anything we send -- worth knowing before someone goes hunting for a
    bug in the gain path

Nothing was found past 0x10, but nothing rules it out either; the list
is still recorded as possibly partial.

Refs Patola#7

Co-Authored-By: Claude <noreply@anthropic.com>
With Trackball Mode on, the sensitivity sliders did nothing at all on a
DS4 right stick.

A ball on a self-centering stick is meaningless -- it works on position
deltas, so holding the stick still stops the pointer and releasing it
drags the pointer back -- so BallModifier.whole passes RIGHT straight
through when that slot carries stick data. But SensitivityModifier walks
down to the first action with set_speed() and stops there, which on a
trackball binding is the ball, and the ball applies its speed only in
_add and _roll: both on the path it just skipped. The speed landed on a
modifier that had already stepped aside, and the child stayed at 1.0.

The ball now hands its speed to the child before bypassing, multiplied
into whatever the child already has (so it is right whichever side of the
ball the sens() ended up on) and once only, since set_speed is absolute.
It must not happen when the ball is really running, or the speed would be
applied twice; that is asserted.

Also: BallModifier.compress overrode Modifier.compress and dropped its
child-recursion, so a sens() written INSIDE a ball survived as a live
SensitivityModifier -- which has no whole(), so the binding did nothing
whatsoever. The GUI always writes sens() outside, so only hand-written
profiles could hit it. Restored, keeping the ball(circular(...))
turn-around it overrode compress() for in the first place.

NOTE: the compress change affects every controller, not just the DS4.

Refs Patola#5

Co-Authored-By: Claude <noreply@anthropic.com>
The Strength slider stopped at 32639 instead of 32767, which reads as an
off-by-something in the code rather than as a limit -- and it silently
capped the range every haptic strength measurement was taken over.

A GtkAdjustment's usable maximum is upper - page_size, and page_size means
nothing for a slider. adjFAmplitude and adjFPeriod both carried 128.

Guarded by pinning the amplitude slider's range to the driver's own
HAPTIC_AMPLITUDE_MIN/MAX rather than banning the property outright: some
adjustments use it deliberately (global_settings sets upper 10.01 with
page-size 0.01 to land on a round 10.00).

Co-Authored-By: Claude <noreply@anthropic.com>
README: a DS4/DS5 cannot be claimed exclusively over Bluetooth. A Steam
Controller is taken by claiming its USB interface, which detaches the
kernel driver; a PlayStation pad over Bluetooth is reached through
/dev/hidrawN, and hidraw allows several readers at once. With Steam
running, Steam and SC Controller both receive every report and both act on
it, and no code of ours can prevent that.

Written symptoms-first, because the failure does not look like a conflict:
the touchpad glides with an acceleration nobody configured, its click
gives a mouse button whatever you bind, and yet your other bindings work
fine -- so it reads as half-broken configuration rather than as a second
program driving the pad. That misreading cost three rounds of debugging
here before someone noticed Steam holding the hidraw node.

TODO: haptic feedback on the DS4 is not implemented at all -- all three
DS4 controller classes inherit a no-op feedback(), and EvdevController's
is an empty TODO docstring. ds5drv.feedback is close to a template, with
two things not to copy blindly, and Controller.rumble() is unimplemented
for both pads, which is why fftest's heavy and light effects feel
identical on them.

Co-Authored-By: Claude <noreply@anthropic.com>
The daemon was calling os.umask(0) after daemonizing, which left all
subsequently created files (including the IPC socket) world-accessible.
Change to os.umask(0o077) so files are owner-only by default.

For the IPC socket, call os.chmod() to enforce 0600 immediately after
bind() and before serve_forever() starts accepting connections,
ensuring there is no window where the socket is accessible to other
users.

Assisted-by: Claude Opus 4.6
Signed-off-by: Sergio Correia <scorreia@redhat.com>
The Bluetooth long-packet reassembly extracts a 4-bit packet
number from the incoming data (range 0-15) and uses it to compute
a write offset into a 256-byte buffer. For packet numbers >= 14
the offset exceeds the buffer, and the subsequent memcpy corrupts
adjacent struct fields on the heap.

Reject packets whose computed offset would overflow the buffer.

Confirmed by walking the arithmetic: with PACKET_SIZE 20 the offset
is 18n + 2, so packet 14 writes to 272 and packet 15 to 290, past
the end of a 256-byte buffer -- 16 and 34 bytes of heap corruption
driven by a nibble a paired device chooses.

[patola: offset made size_t. As submitted, the bounds check compared
 an int against sizeof and warned under -Wsign-compare, which is not
 where you want a signedness question. Restored the two comment lines
 the patch replaced -- they explain the +2.]

Assisted-by: Claude Opus 4.6
Signed-off-by: Sergio Correia <sergio@correia.cc>
Co-Authored-By: Claude <noreply@anthropic.com>
The CLAMP macro was defined as a no-op (expanding to just x),
providing no actual clamping. Replace with a correct implementation.

Add a data_len parameter to grab_value/grab_with_size and validate
byte_offset against it before reading, preventing out-of-bounds
reads from malformed HID reports. Guard the HATSWITCH case against
i+1 overflowing the axes array.

Recalibrate DS4/DS5 axis parameters to work with the real CLAMP:
the old values repurposed clamp_max as a scale factor (e.g. 257),
which only worked because CLAMP was a no-op. Normalize fval to
[-1, 1] (sticks) or [0, 1] (triggers) and use STICK_PAD_MIN/MAX
and TRIGGER_MAX as proper clamp bounds, matching the evdevdrv
convention. Set decoder.packet_size in DS4/DS5/BT constructors
so the new bounds check has the correct buffer length.

Assisted-by: Claude Opus 4.6
Signed-off-by: Sergio Correia <scorreia@redhat.com>
The IPC socket accepted arbitrary action strings in Replace: commands,
including shell() which executes commands via subprocess. The Profile:
and Selected: handlers accepted arbitrary filesystem paths, allowing
any socket client to load attacker-controlled files as profiles or
menus containing shell() actions.

Add SafeTalkingActionParser that filters dangerous action types
(shell, profile, restart, exit, turnoff) from Replace: commands,
including in nested/composite actions. The override changes the
default parameter so recursive _parse_action() calls also use the
filtered dict.

Reject Profile: and Selected: names containing '/' and resolve them
through find_profile()/find_menu() which only search known safe
directories.

Note what this filter is and is not. It is defence in depth, not a
security boundary: type() and button() have to keep working for
Replace: to have any purpose, and type('...') plus Enter drives
whatever terminal happens to be focused. The boundary is the socket
being 0600 (see the umask commit); this narrows what a client that
already has that access can do without effort.

[patola: dropped the realpath containment check in find_profile and
 find_menu. The basename check already stops traversal completely, and
 resolving symlinks additionally rejected a profile that is a symlink
 to somewhere else -- how people keep them in a dotfiles repo -- while
 only defending against a symlink planted inside the profiles
 directory, where an attacker could equally just write a profile full
 of shell() actions. Same reasoning in the daemon's two path checks:
 abspath instead of realpath, which collapses '..' lexically so
 traversal still cannot escape.

 Rejecting '.' and '..' explicitly, because basename('..') == '..'
 passes the basename check and, for menus, resolves to the directory
 itself. The submitted realpath check caught that by accident; the
 lexical one has to say so.]

Assisted-by: Claude Opus 4.6
Signed-off-by: Sergio Correia <sergio@correia.cc>
Co-Authored-By: Claude <noreply@anthropic.com>
Replace os.system() in on_sa_restart with subprocess.Popen using a
list and find_python(), eliminating shell injection via sys.argv[0]
and ensuring the correct Python interpreter is used in AppImage
environments.

Fix Task.__lt__ which was a no-op (missing return). Add a sequence
number as a tiebreaker so equal-timestamp tasks maintain FIFO order
and PriorityQueue never falls through to comparing incomparable
callback objects.

Assisted-by: Claude Opus 4.6
Signed-off-by: Sergio Correia <scorreia@redhat.com>
The header lookup ended in an unguarded `else`, so a machine with neither
kernel headers nor the bundled copy did not get a diagnosis -- it got
FileNotFoundError on /usr/include/linux/input.h, a path it had never
been told to care about, raised from inside an import three levels down.
That is exactly the state a build sandbox is in, and reading it from the
far end cost a round trip.

The three candidates move into _find_event_codes_header(), which raises
ImportError naming every path it tried and how to satisfy it. `exists`
is injectable so the empty case is testable without a machine in that
state; the preference order is unchanged.

Co-Authored-By: Claude <noreply@anthropic.com>
has_haptic_effects() asked whether the connected controller's type is one
that can synthesise waveforms. With no controller connected the type is
None, which is not in that set, so the Effect row disappeared from the
Feedback tab -- reproducible by opening a pad's configuration with the
Steam Controller 2 switched off.

There are three states, not two: capable, incapable, and nothing
attached. An absent controller says nothing about the hardware a profile
is written for, and editing a profile with the pad off is ordinary. Treat
unknown as capable; a known incapable controller still hides the chooser.

Saved effects were never at risk -- update_modifiers skips the effect
block when unsupported and _make_action rebuilds from the loaded state,
so a Tone opened offline stayed a Tone. There is now a test saying so,
because that was the failure worth ruling out.

Reported by Patola on a Steam Controller 2.

Co-Authored-By: Claude <noreply@anthropic.com>
ReplacedAction.button_press passed mapper as both arguments instead
of forwarding the call correctly, causing TypeError on every button
press through a Replace'd action.

ReportingAction.whole had a misplaced parenthesis that evaluated the
comparison inside abs() instead of outside, defeating the Y-axis
minimum-difference threshold for event filtering.

ReplacedAction.__init__ also bypassed LockedAction.__init__, which
meant the held-button-release guard was skipped: if a button was
physically held when Replace: was issued, the old action's virtual
key stayed stuck down. Add the same release-if-held guard.

Assisted-by: Claude Opus 4.6
Signed-off-by: Sergio Correia <sergio@correia.cc>
force_restart() appended a bare (vendor_id, product_id) tuple to
_retry_devices, but the retry loop unpacks each entry as
(syspath, (vendor, product)). The mismatch raised ValueError,
crashing the USB mainloop and killing communication with all
controllers.

Store syspath on USBDevice instances when they are successfully
handled, and include it in the retry tuple.

Assisted-by: Claude Opus 4.6
Signed-off-by: Sergio Correia <sergio@correia.cc>
__mul__ passed self.frequency (already scaled by 1000x in the
constructor) back through the constructor, which multiplied by
1000 again. This also silently dropped extended effect attributes
(tone, sweep, LFO settings).

Apply the same pattern as with_position(): construct with a dummy
frequency, then copy the pre-scaled frequency and all extended
attributes directly. Extract the shared attribute list into a
class-level _EFFECT_ATTRS tuple used by both methods.

Assisted-by: Claude Opus 4.6
Signed-off-by: Sergio Correia <sergio@correia.cc>
RangeOP for both STICK and RSTICK created children that resolved
to lpad_x/lpad_y axis names, so modeshift conditions on STICK
read left-pad state instead of stick state, and RSTICK conditions
never matched the right stick at all.

Override children's axis_name to stick_x/stick_y and rstick_x/
rstick_y respectively, and guard _state() against AttributeError
for controllers whose state struct lacks the requested axis (every
cmp_* already treats None as no-match).

[patola: took only the actions.py half of the submitted patch. The
DS5 Bluetooth touchpad scaling it also carried is held back: it
scales unconditionally, where the USB path it copies only scales
inside the touched branch, so a lifted finger would report a pad
corner instead of centre -- and it scales a cpad_y that is itself
extracted wrongly ((data[37] & 0x0F) << 4 keeps 8 bits of a 12-bit
field). That needs the extraction fixed first and a DualSense to
test on.]

Assisted-by: Claude Opus 4.6
Signed-off-by: Sergio Correia <sergio@correia.cc>
The button_map guard allowed values up to 32 (< 33), but shifting
a uint32_t by 32 is undefined behavior in C. Change the guard to
< 32 so only valid bit positions 0-31 are used.

Assisted-by: Claude Opus 4.6
Signed-off-by: Sergio Correia <sergio@correia.cc>
The _threaded method and kill() both read and write self.p without
synchronization. If kill() checks self.p between _threaded's
communicate() return and self.p = None assignment, it may call
kill() on None or on a stale Popen. Add a threading.Lock to
protect self.p access.

Also re-check _killed inside the lock before spawning, closing a
narrow TOCTOU between the while-guard and the lock acquisition
that could leave an orphaned child process after kill() returns.

Assisted-by: Claude Opus 4.6
Signed-off-by: Sergio Correia <sergio@correia.cc>
The "maps to nothing" sentinel was BUTTON_COUNT - 1. That is bit 31,
which is RSTICKPRESS -- a real button -- so on a JSON-configured HID
controller every input bit the config did not name pressed the right
stick, and button_to_bit reported the same thing for a mask it could not
resolve.

Use BUTTON_UNMAPPED = BUTTON_COUNT instead, which is outside the range
the decoder can turn into a button. That value is only safe now that the
preceding commit rejects 32 rather than 33: before it, an out-of-range
sentinel would have been shifted into a uint32_t by 32 -- undefined
behaviour rather than the intended no-op.

Found while reviewing Sergio Correia's shift-UB fix, which this builds on.

Co-Authored-By: Claude <noreply@anthropic.com>
Two faults that compounded into data loss. save() wrote in place, so
anything killing the process mid-write left truncated JSON; reload()
then answered the parse failure by calling create(), which overwrites
the file with defaults. One interrupted write therefore discarded every
setting the user had, with no copy kept.

save() now writes a temp file in the same directory, fsyncs it and
os.replace()s it in, so a reader sees either the old contents or the
new. reload() moves an unreadable config aside to config.json.broken.N
before falling back to defaults, numbered so a second failure cannot
overwrite the copy taken during the first -- which would be the only
surviving record of the settings. A missing file is treated as a first
run rather than an error, so it no longer logs a warning or leaves a
stray backup.

Also: "Minimize to status icon instead closing" -> "instead of closing".

Reported by Patola, who lost his settings twice; the fingerprints
matched exactly -- minimize_to_status_icon and autokill_daemon sitting
at their defaults, news.last_version reverted from 0.4.8 to the default
0.3.12, and osd_color_theme (which has no default) gone entirely.

Co-Authored-By: Claude <noreply@anthropic.com>
Navigating an OSD menu with a DualShock 4's left stick was unusable:
nudging it scrolled the list far too fast, and holding it fully deflected
stopped the selection dead. The d-pad on the same pad was fine, and both
were fine on an SC2.

One cause, two symptoms. Menus accept STICK, LEFT and DPAD together --
the d-pad arrives as LEFT on a DS4 and as DPAD on an SC2 -- and all of
them fed one StickController. _move() emits on every direction change and
cancels the repeat timer once the direction reaches zero, so the idle
input reporting (0, 0) cancelled the repeat the stick was driving.
Movement then happened only on changes: once per event, at the
controller's report rate, hence "too fast" -- and nothing at all once the
stick was held still and stopped producing events, hence "stalls".

set_stick now takes the input the position came from and keeps one entry
per source, letting whichever is actually deflected decide. Callers with a
single input (launcher, dialog) pass no source and are untouched.

Pre-existing: LEFT has always been accepted here. The DPAD added in
v0.6.0.7 only affected controllers with a real separate d-pad, which are
the ones that behaved correctly.

Also makes the packet_size tests from the security series skip rather
than fail where libhiddrv is not built. They went in red -- CI has been
failing since that merge, because I ran the suite only on a machine that
has the extension.

Fixes Patola#17

Co-Authored-By: Claude <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants