Repository navigation
Conversation
- It reads the controller and puts the named button or
axis value into memory as "/event/button/{name}" or
"/event/axis/name".
- The controller can be given a dictionary to map low level
button and axis names to some more generic name. If
no map is provided then the low level names are used.
- The event is deleted after one pass through the event loop.
- This is part of controller rewrite. it is the base system
for reading raw joystick values and getting them
into vehicle memory.
- remove() takes a list of keys to delete from memory. - add type to Vehicle constructor
- Buttons now output press/release/click one-shot events. The click event includes the count of sequential fast-clicks that can be used to support double-click, triple-click, etc. - The value of a button event is the time at which the event occured. - The value of an axis event is the value of the axis. - The state of a control is now written to memory a persistent value so that parts can use them as inputs. This allows a part to use one control's value as a modifier to another control's event. It also allows a part to use an control's state without having to listen for events and maintain the control's state itself.
First commit of the game-controller event refactor (#1097); see CONTROLLER_EVENTS_PLAN.md, task 0.1. - Memory.remove() ignores keys that are not in memory. The emitting part owns event expiry, so it will routinely try to remove a key that another part has already removed; `del` raised KeyError there. - Rewrite donkeycar/events.py as OneShotEvents, the bookkeeping helper that publishes events into memory and expires them on the next emit, so an event is visible for exactly one full pass through the vehicle loop. Both InputControllerEvents and the later BehaviorEventMapper build on it. This replaces the unused Events class, whose expire_events() mutated a dict while iterating it and dropped events a tick early. - Type memory.py fully and add a scoped strict-mypy config plus a CI step. Strictness is opted into per module via `files` in pyproject.toml as the refactor lands, rather than turned on repo-wide. - Fix a latent Memory.__setitem__ bug: for a non-tuple sequence key it ran `value = tuple(key)` where `tuple(value)` was meant, storing the keys as the values. The existing test passed a tuple key and took the other branch, so it never caught this. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Task 0.2 of the game-controller event refactor (#1097); see CONTROLLER_EVENTS_PLAN.md. Introduces donkeycar/parts/controls/ with the device contract every input device implements, and the Linux joystick implementation of it: - ControlChange, a NamedTuple replacing the bare four-tuple poll() used to return. It still unpacks as that four-tuple, so the older call sites keep working, but the fields now have names and types. (The previous annotation had `str | ModuleNotFoundError` for the axis, a typo.) - AbstractInputController, the abstract base a device must derive from. - A JsDevice protocol between LinuxGameController and the operating system. This is what makes the per-gamepad commits testable: init() was previously nothing but fcntl.ioctl calls, so CI -- which has no /dev/input/js0 -- could not exercise a name map at all. LinuxJsDevice is the real implementation; FakeJsDevice replays scripted events in tests. Defects fixed along the way: - The button and axis name maps were mutable default arguments that init() wrote into, so one controller's names leaked into the next. They are now copied into instance state. - A subclass assigned its maps after calling super().__init__(), silently discarding any names the caller passed. Subclasses now declare BUTTON_NAMES and AXIS_NAMES as class constants and caller-supplied names layer on top, which is what lets a user rename one control from myconfig.py. - Added axis_epsilon, a deadband against stick jitter measured against the last reported value so slow sustained movement still accumulates through. A return to exact center is always reported. - An event for a control the driver never declared is logged and ignored rather than raising IndexError. donkeycar/tests/fake_js.py carries the fixture the twelve gamepad commits share, including duplicate_literal_keys(), which reads a class's source because a duplicate dict key cannot be found in the resulting dict -- Python has already dropped the earlier entry. The helpers have self-tests, and the module is type-checked so the fake cannot drift from the JsDevice protocol. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Task 0.3 of the game-controller event refactor (#1097); completes Phase 0. See CONTROLLER_EVENTS_PLAN.md. InputControllerEvents polls an AbstractInputController on a background thread and publishes what it reads into the vehicle memory: one-shot events that live for exactly one pass through the loop, so they can drive a part's run_condition, and persistent control state, so a part can read a button or axis without tracking the events itself. Clicks are now deferred (decision D1). The proof of concept published '/click/1' the moment the button came up and '/click/2' on the next fast release, so every single-click behavior also fired partway through every double-click. A click is now held until the burst ends, and only the final count is published. The cost is fast_click_time of latency on clicks; press and release stay immediate. Pass defer_clicks=False for the old behavior. A press held past long_press_time now publishes a hold (decision D2), and that press does not go on to publish a click -- a long press is a gesture of its own, not a slow click. Long-click *counting* is deliberately left out until something asks for it. Also fixed, from the proof of concept: - The clock is injected, so click and hold timing is tested without any real waiting, and it defaults to time.monotonic rather than time.time. - The bare `except:` in poll() and update() caught KeyboardInterrupt and silently stopped the controller with nothing in the log. poll() now handles OSError, the disconnected-device case, and update() logs a traceback for anything else. Both let KeyboardInterrupt through. - The device was opened from the constructor, which blocked assembling the vehicle for five seconds on an unpaired gamepad and then failed the whole car. The retry loop moved into the threaded update(). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Task 0.4 of the game-controller event refactor (#1097). Both defects were found running Phase 0 against a real Xbox One S pad on the car; neither is reachable through FakeJsDevice, which returns immediately and never blocks. Shutdown deadlocked against its own reader. Reads went through a buffered file object whose peek() blocks on an idle gamepad -- which is nearly always -- while holding that object's internal lock, so close() from the vehicle thread waited on a reader that was itself waiting on the pad. A thread dump showed the reader parked in read_event and the main thread parked in close. Reads now go through a raw non-blocking descriptor, which has no such lock. A vanished device was reported as "no event", forever. The pad dropped off USB and re-enumerated; the reader kept the old descriptor and returned "nothing happened" every 50ms for the rest of the run, with no error, no log line, and every button press going nowhere. That is the same silent failure the bare `except:` used to cause. select.select() cannot tell a quiet device from a gone one, so reads use select.poll(), which reports POLLHUP, POLLERR and POLLNVAL; a vanished device now raises OSError, which InputControllerEvents already knows how to log and stop on. It stays silent when the descriptor is gone because we closed it, since that is shutdown rather than failure. The new tests drive a real os.pipe() rather than the fake: one parks a reader in poll() and asserts close() returns promptly, the other closes the far end and asserts the disconnect is raised. Also records in the plan what the pad actually reported, because the legacy XboxOneJoystick axis map is wrong for the xpad driver: 0x02 and 0x05 are the triggers, not the right stick, and the right stick at 0x03/0x04 has no name at all. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Reference for task 1.7 of the game-controller event refactor (#1097). Each control was moved in isolation with pauses between, against a real Xbox One S pad through the xpad driver, with every axis event timestamped, so the mapping below is attribution rather than inference from convention. The legacy XboxOneJoystick axis map is wrong for this driver. 0x02 and 0x05 are the left and right triggers, not the right stick; the right stick is at 0x03 and 0x04 and has no name at all; and 0x09 and 0x0a, where the legacy map looks for the triggers, are not reported by the device. Triggers rest at -1.0 and travel to +1.0 while sticks rest at 0.0 and travel both ways, which is what tells them apart in the capture. That also makes the legacy Forza mode dead code here: magnitude() is bound to the names right_trigger and left_trigger, which map to codes the device never sends, so it never runs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Task 1.7 of the game-controller event refactor (#1097), taken out of order because the pad was on the bench and a map written from measurement is worth more than one written from convention. Starts donkeycar/parts/controls/gamepads.py, where a controller is pure data: a LinuxGameController subclass naming the driver's control codes. The legacy XboxOneJoystick map was wrong on four of eight axes. Each control was moved in isolation against a Microsoft X-Box One S pad on the xpad driver, with the axis events timestamped, which gives: 0x02 left trigger legacy called it right_stick_horz 0x05 right trigger legacy called it right_stick_vert 0x03 right stick X legacy left it unnamed 0x04 right stick Y legacy left it unnamed 0x09, 0x0a legacy named these; the device never sends them So on main today an Xbox user's "right stick" is really their triggers, which sit pegged at -1.0 whenever they are not squeezed, and the real right stick has no name. Legacy Forza mode is dead code for the same reason: magnitude() is bound to the names right_trigger and left_trigger, which resolve to codes this device never reports. Four buttons were also unnamed: view, xbox, and the two stick presses. The tests pin the enumeration the real pad produced, so they check the map against the device rather than only against itself -- a map can be perfectly self-consistent and still describe a different controller. That is the one thing the duplicate-code and duplicate-name assertions cannot catch, and it is exactly how the legacy map went wrong. Verified on the car: every control the driver reports now resolves to a real name, with no 'axis(0x..)' or 'button(0x...)' fallbacks left. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Task 1.1 of the game-controller event refactor (#1097), re-landing the proof of concept's map on the new base. Also drops XboxOneSwapped from the plan: it only swaps which stick steers and which drives, which is a behavior binding rather than a device naming, so it belongs in the Phase 4 behavior map instead of being a controller class. No F710 was available, so unlike the Xbox map this one is not verified against hardware and the docstring says so. The tests cross-check it against the pad that was measured instead: an F710 with its mode switch on X presents as an Xbox 360 controller and is driven by the same xpad driver, and one driver cannot report the same code as two different controls, so every code the two maps share must mean the same thing on both. Reintroducing the legacy Xbox defect into this map fails that check, so it is not inert. That check also corroborates the 1.7 measurement from the other direction: the map Donkeycar has shipped for the F710 all along puts the triggers at 0x02/0x05 and the right stick at 0x03/0x04, which is exactly where measuring the Xbox pad put them, and exactly where the legacy Xbox map did not. Names are normalised to the Xbox map for controls of the same kind, so the triggers are left_trigger and right_trigger rather than L2_pressure and R2_pressure -- PlayStation labels on a pad that prints LT and RT. The three buttons whose labels really do differ keep the Logitech words: back, start and logitech. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Task 1.2 of the game-controller event refactor (#1097), for the in-kernel hid-sony driver. PS3 pads are unusually driver-dependent, so the sixad and older Jessie mappings stay separate classes rather than becoming options here. Two things differ in shape from the Xbox-style pads, and the tests pin both: the dpad is four buttons rather than a pair of axes, and each trigger is reported twice over, as an analog axis and as a digital button, so a squeeze produces both. Both are named so a part can use whichever it needs. No DualShock 3 was available, so this is not verified against hardware and the docstring says so. The axis cross-check against the measured Xbox pad is weaker here than the one used for the F710: that pad shares the xpad driver, whereas this only shares the kernel-wide ABS code convention, which hid-sony happens to follow but is not obliged to. The test says as much rather than implying more confidence than it earns. Also settles the naming convention for the rest of Phase 1, in the gamepads module docstring. Controls every pad has are named for what they are and named identically across pads, so a behavior map reads the same whichever controller it is for; controls carrying the pad's identity keep the words printed on it, so cross and select on a PlayStation pad, a_button and menu on an Xbox one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Task 1.3 of the game-controller event refactor (#1097): the same DualShock 3 as 1.2, through the sixad userland driver used on the Jetson Nano. This is the pad that shows why a map is written against a driver rather than against hardware. It shares just two axis codes and one button code with the in-kernel map for the very same controller. sixad builds its own uinput device and numbers the axes sequentially -- left stick, then right stick -- so the right stick lands on ABS_Z and ABS_RX, the codes that mean the triggers on every in-kernel driver. It also reports no analog triggers at all; L2 and R2 arrive only as digital buttons, so proportional trigger control does not exist here. And its buttons sit in the generic joystick block rather than the gamepad block, because sixad presents the pad as a plain joystick. The tests assert those disagreements on purpose. A later reader comparing this map against the others would see the right stick on "the trigger codes" and be tempted to correct it, which would move steering onto a control the driver never sends. Better to have a test say the difference is deliberate. Also factors the repeated map-soundness assertions into GamepadMapChecks in the shared test kit, this being the fourth pad to copy them, and drops the "every control is named" test from the two maps that had no hardware behind it. Built from a map's own codes that test cannot fail, and a test that cannot fail is worse than no test: it reads like coverage. It stays in the Xbox tests, where the codes come from a real device. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Task 1.4 of the game-controller event refactor (#1097): the DualShock 3 on the Raspbian Jessie-era hid-sony driver. A third layout for the same controller, agreeing with neither of the others. The right stick sits at 0x02/0x05 here, 0x03/0x04 through the modern in-kernel driver and 0x02/0x03 through sixad; the PS button is at 0x2c0, 0x13c and 0x130 respectively. The tests state all three side by side, since one pad with three incompatible layouts is the clearest argument that these maps belong to a driver rather than to a piece of hardware. What distinguishes this driver is that it exposes the DualShock 3's pressure sensitivity: every face button, shoulder and dpad direction reports how hard it is pressed, as an axis alongside the ordinary digital button. Twelve of the twenty axes are pressure. The legacy map named eleven of those twelve. They run consecutively from 0x2c to 0x37 in the order the pad reports them -- dpad, shoulders and triggers, then the face buttons -- and 0x2f was the single gap, so a left-dpad press arrived as an anonymous axis(0x2f) that nothing could bind. It is named here, and a test asserts the run has no gaps so a later omission is caught. The inference is from the sequence rather than from a device, and the docstring says so. The two analog triggers take the shared left_trigger and right_trigger names even though this pad calls them L2 and R2 pressure, so that a behavior map reads the same whichever controller it was written for. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Task 1.5 of the game-controller event refactor (#1097), completing the PlayStation 3 variants. Unlike the other three this is not a different layout. Every code it shares with PS3Joystick means the same thing -- the buttons are identical, and so are the sticks and the triggers -- and it adds the twelve pressure axes and four tilt axes on top. So it derives from PS3Joystick rather than restating it, which also keeps the two from drifting apart if either is corrected, and the tests assert that relationship holds rather than leaving it as a comment. That leaves a question for someone with the hardware. If this really is the same driver with more axes surfaced, then PS3Joystick is not different but incomplete, and a Pi user is getting anonymous axis(0x2c) events for pressure their pad is actually sending. If so the two should merge. Nobody has confirmed it either way, so the docstring asks rather than assumes. As in the Jessie-era map the legacy table left dpad_left_pressure at 0x2f unnamed, and it is filled here on the same reasoning. 0x30 and 0x31 are deliberately left alone: this driver reports the analog triggers at 0x02/0x05 with the sticks rather than in the pressure block, so filling those two by analogy with the other map would name controls this driver never sends. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Task 1.6 of the game-controller event refactor (#1097), and the commit the duplicate-key check was built for. The legacy map wrote 0x13a twice, as both 'L3' and 'share', and 0x13b twice, as both 'R3' and 'options'. Python keeps the last of a repeated key, so both stick presses vanished silently: controls a user can see on the pad and could never bind. Running the check against the legacy class confirms it reports exactly those two codes, so the defect is caught rather than merely fixed. Fixing it meant deciding what the codes really are, and they are not in doubt: 0x13a is BTN_SELECT and 0x13b is BTN_START, so share and options, while the stick presses are BTN_THUMBL and BTN_THUMBR at 0x13d and 0x13e -- which is what the PS3 map on the same driver already does. Two neighbouring errors came out with it. The legacy PS4 and PS3 maps disagree over which codes are the shoulders and which the triggers; they cannot both be right about hid-sony, and the kernel headers side with the PS3 map, so L1 is at 0x136 and L2 at 0x138 here. And 0x13d was named 'pad' for the touchpad click when it is BTN_THUMBL; the touchpad presents as its own input device and never appears on the joystick node, so a behavior bound to 'pad' would have waited forever. No DualShock 4 was available. Every correction follows the kernel's own button codes and agrees with the PS3 map on the shared driver, but a pad on a bench would settle it, and the docstring says as much. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Task 1.8 of the game-controller event refactor (#1097). Resolves the two placeholder axis names that have been shipping in this map: 'hmm' and 'what' are 0x10 and 0x11, ABS_HAT0X and ABS_HAT0Y, which is the dpad -- the same codes measured as the dpad on a real Xbox pad. They are named dpad_horiz and dpad_vert here, matching every other pad with a hat, and a test asserts no placeholder names remain. A control nobody understood is a control nobody will bind. The buttons run consecutively from 0x130 to 0x137 because hid-generic numbers them in the order the HID descriptor lists them rather than by what each one means. So they do not line up with the Xbox-style pads -- 0x133 is Y here and X there -- and the tests assert that disagreement deliberately, as they do for sixad. Correcting it into agreement would shift every face button by one. This driver reports six axes, two sticks and the hat, so the four shoulders are all digital and are named as buttons. A behavior map binding throttle to left_trigger on this pad would bind to nothing, and a test says so. Not verified against hardware. The Nimbus also has a Menu button that the legacy map did not name and this one does not either, because its code is unknown; it will surface as button(0x...) and show_map() will reveal it on a real device. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Task 1.9 of the game-controller event refactor (#1097). Fixes the known typo -- legacy wrote 'PAD_DOWN,' with the comma inside the string, so the event key carried a stray comma -- and a worse defect sitting next to it. Legacy put dpad down on 0x224, which is not a dpad code at all. The four directions are 0x220 to 0x223 and legacy had the other three right, so as it stood dpad down could never fire and 0x221 went unnamed. Down moves to 0x221 and a test asserts the run of four is unbroken. Nintendo arranges the face buttons the opposite way round from everyone else: B is the bottom button and A the right one. The codes follow the position rather than the letter, so a_button sits at 0x131 here where an Xbox pad has b_button. That is correct, and it is asserted deliberately because it looks exactly like the sort of transposition that is usually a bug. Less certain than the other maps. The legacy version was transcribed from a third-party config file and shipped with the comment "need testing!" attached, and no Pro Controller was available to settle it. The docstring says so. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Task 1.10 of the game-controller event refactor (#1097). Not a gamepad, and named accordingly. Its two axes are the transmitter's steering and throttle channels rather than sticks belonging to a hand, so they take those names -- what the control is, which is what this module's convention asks for. A behavior map for it reads directly: throttle binds to /axis/throttle. The third channel is a switch, reported as two buttons for its two positions rather than one button with a state, so a behavior should treat each position as an event rather than tracking a single button. It shares no control name with any gamepad, and a test says so. There is no dpad, there are no face buttons and nothing is analog beyond the two channels, so most of what a template binds by default does not exist here. That is the case the per-controller behavior map has to cope with in Phase 4, and it is better to have it stated now than discovered then. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Task 1.11 of the game-controller event refactor (#1097). CustomJoystick declares nothing at all, which is the point: an unsupported pad has to be usable before anyone can name anything on it, because you cannot write the map until you can see what the device sends. Every control keeps the default name built from its driver code, so show_map() tells the user exactly what to put in myconfig.py, and the pad works meanwhile -- just unreadably. The same layering renames a single control on a pad that does have a map, without restating the rest of it or forking a class, and the tests cover that too since it is the more common need. This is the mechanism task 6.2 turns `donkey createjs` into: the wizard's job becomes producing a name dictionary rather than generating a controller class. That completes Phase 1. Eleven controllers, of which one was measured against real hardware and ten were not, every one saying which it is. The maps between them fixed six defects that were live on main: the Xbox axes (four wrong, two named for codes never sent), the PS4 duplicate keys costing both stick presses, the PS4/PS3 disagreement over shoulders and triggers, the Wii U dpad down on a code that is not a dpad code, the PS3 pressure gap at 0x2f, and the two Nimbus placeholder names. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Task 2.1 of the game-controller event refactor (#1097), starting Phase 2: game controllers on hosts where /dev/input/js0 is not the way in. PyGame does not hand out input events. It exposes where every control currently is and leaves the caller to notice what moved, so one scan can turn up several changes at once. The legacy version walked every control while overwriting a single pair of return values, so when two moved together it reported the last and dropped the rest -- having already recorded their new positions, so the dropped ones were never reported at all. A stick moved diagonally changes two axes at once, so this happened on essentially every scan. poll() now drains a queue and a scan refills it, and three tests fail if that is undone. The seam is the same shape as the one used for the Linux devices: a PyGameDevice protocol with a fake behind it, so the controller and its maps are testable on a host with neither PyGame nor a pad. Hats have no buttons of their own, so each is expanded into four pseudo-buttons -- left, right, down, up -- numbered after the real ones. PyGamePS4Joystick is the same controller as PS4Joystick indexed by PyGame rather than by Linux input codes, so the two share no numbering whatever: the right stick is at axes 2 and 3 here and 0x03/0x04 there. They do share a vocabulary, so a behavior map carries across even though the numbers cannot, and a test holds that line. PyGame also surfaces the touchpad click as an ordinary button, which the Linux driver never does. The module is pygame_device.py rather than pygame.py so that a traceback does not require knowing which of the two you are looking at. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Task 2.2 of the game-controller event refactor (#1097). The legacy part read three GPIO channels and then decided the pilot mode and whether to record, all in the same place, binding two behaviors to fixed channels. That is precisely what made the old controllers impossible to remap, so this reports the channels and stops there: steering, throttle and switch, each as an axis over the same -1.0 to 1.0 range a gamepad stick uses, so nothing downstream needs to know the input came from a radio. The channel names match RC3ChanJoystick, which reaches the same transmitter through a joystick driver, so a behavior map mostly carries between the two routes in. One difference stays deliberate and is tested: that driver thresholds the third channel into two buttons, one per switch position, while this layer reports the raw channel. Thresholding is a decision about how a particular transmitter is wired, so it belongs in a behavior part. pigpio updates pulse widths from its own edge callbacks, so like PyGame this reports positions rather than events, and poll() drains a queue that a scan refills. Steering and throttle moving together -- a driver turning and accelerating at once, which is most of driving -- keeps both. A channel that has never been seen reads zero pulse width and is reported as nothing rather than as centre, since a transmitter switched off is not a driver holding steady. Fixes a defect in the legacy shutdown: it indexed its list of callbacks with a Channel object rather than an index, so it raised TypeError every time it ran, and the pigpio connection was never released either. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Task 2.3 of the game-controller event refactor (#1097). The MM1 does the radio work on its own microcontroller and sends two pulse widths per line down a serial port. This reports them as the axes steering and throttle over -1.0 to 1.0, the range every other controller here uses, and stops there. As with the pigpio receiver, the legacy part also decided whether to record, which is a behavior and does not belong beside the driver. The arithmetic is pinned against the exact readings the existing RoboHAT tests use, so this reports precisely what the legacy part did. An MM1 car's steering feel must not change quietly, and the legacy test still passes untouched. Steering keeps its inversion -- a rising pulse turns left -- and each half of the travel is scaled to its own side of centre, so a receiver whose centre is off-centre still reaches full lock both ways. Two parsing fixes. The legacy version split each line on ', ' exactly, so a firmware that sent '1500,1500' without the space produced a line that parsed to nothing at all, silently. And it tested the fields with str.isnumeric(), which rejects '1600.0'. Both now parse. One thing for someone with the hardware. The legacy part mapped throttle through MM1_STOPPED_PWM, MM1_MAX_FORWARD and MM1_MAX_REVERSE in two passes -- raw range into the configured range, then the configured range into -1..1 -- and those two maps cancel exactly. The three settings have therefore never affected throttle at all. That behavior is preserved here rather than changing how every MM1 car drives on a guess, but if they were meant to do something, they never did it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Task 2.4 of the game-controller event refactor (#1097). NetworkedController is an ordinary input controller, so a template binds behaviors to a pad on another machine exactly as it would to one plugged into the car. That replaces the monkey-patch the templates used -- ctr.js = netwkJs -- which only worked because both objects happened to have a poll(). The wire format is now a line of JSON. The legacy format was four space-separated fields, which broke on any control whose name contained a space: the payload split into too many parts and the unpacking raised, taking the receiving thread with it. It also used the string '0' to mean "nothing happened", so a control actually named '0' could never be reported. JSON has a real null and needs no delimiter to be safe. Both ends ship together, so the format change costs nothing. Three defects fixed alongside. A malformed message killed the receiver. The legacy update() unpacked the split payload into four names with no error handling anywhere, so one bad message stopped the thread and the car stopped responding with no way back. A bad message is now logged and dropped. A burst lost everything but its last change. The legacy receiver held one button and one axis in fields and overwrote them on each message, so anything arriving faster than the vehicle loop read it was gone -- and a network delivers bursts by nature. Losing a release leaves a button stuck down. Messages are queued now, and a test walks a press and its release through. Shutdown could not work. It set a flag and slept while the thread sat in a recv() that blocks forever, which is the same shape as the deadlock found on the car in task 0.4. The subscriber now takes a receive timeout. The publisher takes any AbstractInputController rather than hardcoding a PS3 pad, so driving from another controller no longer means editing the class. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Task 2.5 of the game-controller event refactor (#1097), completing Phase 2 and settling decision D3. A web button and a gamepad button were handled quite differently. The web controller latched a push into a dictionary, a part exploded that into memory as 'web/w1', and a template bound to it directly, while a gamepad button went through set_button_down_trigger on the controller object. So every binding in path_follow.py and cv_control.py is written twice, once for each route in, and the two had different ideas of what a press meant. WebButtonController presents those pushes as an ordinary input controller, so a push becomes '/event/button/web_w1/press' and passes through the same event pump as everything else -- which means press, release, click and multi-click come for free, and a behavior can be bound to a web button or a gamepad button without the template knowing which. That is what lets Phase 5 delete the branching rather than porting it. It bridges two loops and so has a foot in each: run() takes the button dictionary from the vehicle loop, poll() is called from the event pump's thread, and a lock and a queue sit between them. poll() waits briefly rather than spinning, and returns so shutdown is noticed. A button that drops out of the dictionary is released rather than left held down. The web controller stops reporting a button once it has cleared the latch, and treating that silence as "still pushed" would leave it stuck down for the rest of the drive. No hold event will come from this controller however long the mouse is held: nothing in the web interface reports a button's duration. The docstring says so rather than leaving someone to find out by binding one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Task 3.1 of the game-controller event refactor (#1097), starting Phase 3: turning JoystickController's methods into parts that can be bound to any control. UserSteering and UserThrottle do what set_steering and set_throttle did. The throttle scale arrives as an input as well as a constructor argument, so AdjustMaxThrottle can change it while driving -- that shared mutable field on the controller object is one of the things that made the old behaviors impossible to separate, and naming it in memory is what replaces it. TriggerThrottle is what the legacy code called Forza mode, and since that was dead code -- bound to axis names mapping to codes the driver never sends -- it has no old behavior to preserve, only correct behavior to establish. Two things follow from that. It takes both triggers together rather than binding each to its own handler. The legacy pair wrote the throttle in turn, so whichever moved last won outright and holding forward while brushing reverse gave full reverse. Taken together, squeezing both cancels, which is the sensible reading of an ambiguous request. And an untouched trigger is treated as released rather than centred. A trigger rests at -1.0, as measured on a real pad, and has no value in memory at all until it first moves. Reading that absence as centre would put the car at half throttle the moment it started, so it is explicit and tested. JOYSTICK_DEADZONE is available on both parts but defaults to off, because the legacy controller applied it only to the decision about whether to record and never to the steering or throttle values. Switching it on changes how a car drives, so a template has to ask for it rather than inheriting it by accident. The measured jitter from a resting stick -- 0.048, 0.068 and 0.099 -- is in the tests, since that is the case a dead zone is for. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The legacy controller applied JOYSTICK_DEADZONE only to the decision about whether to record, never to the steering or throttle values, so a stick resting slightly off centre steered the car and a car with nobody touching it drifted. The driving parts landed in 3.1 with the dead zone available but off, to avoid changing how a car drives without asking; asked, and it is on. The default is 0.01, matching the shipped configuration. Worth knowing that is smaller than the jitter measured on the test pad, which idled between 0.048 and 0.099, so the default stops a tidy pad drifting but not a worn one. A car whose sticks idle that far out wants JOYSTICK_DEADZONE raised in myconfig.py, and the test says as much rather than implying the default handles every pad. Pass dead_zone=0.0 for the old behavior. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Task 3.2 of the game-controller event refactor (#1097). TogglePilotMode and ToggleRecording take the current value as an input and return the next one, rather than holding it themselves. That matters because the mode is not theirs: the web interface changes it, and an emergency stop will drop the car back to manual, so a part that remembered its own idea of the mode would step on from the wrong place. Both step once per call and so want a run condition true for a single pass, which is exactly what an input event is. AutoRecordOnThrottle records while the car is being driven, and only while the human is driving unless asked otherwise -- recording the pilot's own output and training on it teaches the pilot to do what it already does. These replace the ToggleRecording in complete.py, which #1097 asks to have removed. That part decided three things at once through two latches and a remembered previous value: a manual toggle, whether auto-record-on-throttle overrode it, and whether recording was allowed in autopilot. One consequence was that the manual toggle silently did nothing whenever auto-record was on -- it logged a line and ignored the press. Now a template picks either the button or the throttle, and the choice is visible where the parts are added rather than buried in a conditional. The old part stays until 5.1 rewrites the template that uses it. EraseLastNRecords logs and carries on if the tub cannot be written. Erasing is a convenience, and a car that is driving must not fall over because of it. ShowRecordCount no longer needs to hijack the circle button: the legacy template bound it there only when auto-record-on-throttle had made manual recording unavailable, on the reasoning that the button was going spare. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Contributor
|
@DocGarbanzo, I have correctly uncommented the PWM_STEERING_THROTTLE PCA9685 section of I could be doing something wrong that I am not aware of? Ed said he would investigate my issue during the yesterday's Maintainer Meeting, but so far I have received no response from him. TCIII |
Contributor
|
MM1 integration issues at head Missing default MM1 behavior map. Recommended fix in 'MM1': {
STEERING: format_axis_event('steering'),
THROTTLE: format_axis_event('throttle'),
},Default throttle direction reverses forward MM1 input once bindings are supplied. The new MM1 controller converts a serial reading of Recommended fix: preserve MM1's positive-forward convention in the template (e.g. direction +1 for MM1), with explicit/documented treatment of deliberate inversion settings. Reproduction using the PR's
Please add a template integration regression test that injects MM1 serial readings and asserts propagation with no custom behavior map, correct forward/reverse signs, and return to neutral. Conversion-only tests and direct output-driver tests bypass these integration issues. |
Reported on the PR by someone testing an MM1: once its controls were bound, forward stick drove the car backwards at a quarter of its input. Reproduced through the real complete.py wiring, and pigpio_rc does the same. The driving parts took their direction and scale from the JOYSTICK_* settings for every controller. JOYSTICK_THROTTLE_DIR defaults to -1.0 because a gamepad stick reads negative when pushed forward; an RC receiver already reads positive for forward, so the same inversion reversed it. And JOYSTICK_MAX_THROTTLE, 0.5 by default, halved it -- although the transmitter's endpoints, and MM1_MAX_FORWARD on an MM1, already limit how hard it drives. Legacy passed both receivers straight through. get_control_scaling() now answers both questions for a configuration: the JOYSTICK_* settings for a gamepad, and direction +1 with no scaling for MM1 and pigpio_rc. complete.py and simulator.py use it, and basic.py's RC path drops the scaling it applied too. A receiver that genuinely reads backwards is fixed where it always was, on the transmitter or with PIGPIO_INVERT, and the config templates now say the JOYSTICK_* settings are for game controllers. Nothing binds these receivers' controls by default yet, so no car changes behaviour in this commit; the next adds their behavior maps. Phase 8 of the plan is added in this commit, since this is its first task. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Reported on the PR by someone testing an MM1: with the shipped configuration its steering and throttle never reached the car. The templates build both RC receivers themselves rather than through get_input_controller, so neither ever got a default map, and every check that a controller type has one walked CONTROLLER_TYPES and never saw them. Their input reached the event layer and went no further. Both now bind steering and throttle to their channels. pigpio_rc's switch stays unbound: legacy held the pilot mode at 'local' while it was up, and an axis state bound straight to a toggle would toggle on every pass. The reviewer asked for a test that runs the template's wiring end to end, since the conversion tests and the map tests both passed while the car did nothing. test_rc_template_wiring.py feeds MM1 serial lines and pigpio_rc pulse widths through complete.py with the shipped config and no custom map, and checks propagation, forward and reverse signs, and return to neutral. Removing these maps fails it, and so does putting the gamepad inversion back on RC input. A second test reads the CONTROLLER_TYPE options cfg_complete.py offers and requires each to be buildable and, but for custom, to have a map. RC_RECEIVER_TYPES now names each receiver's class, so the existing check that a map binds only controls its pad reports covers these too. That existing check is also why the test added in "Make xbox the Bluetooth map" was a duplicate, and why that commit's claim -- that the Bluetooth map would otherwise have passed every test -- was wrong. The duplicate is removed and the plan says so. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Contributor
Author
Thanks — both confirmed and fixed in 591e156 and 13e9c22; pigpio_rc had the same two problems. MM1 and pigpio_rc now have default maps binding steering and throttle. RC input is no longer flipped by JOYSTICK_THROTTLE_DIR or scaled by JOYSTICK_MAX_THROTTLE/JOYSTICK_STEERING_SCALE, matching legacy — the transmitter's endpoints and MM1_MAX_FORWARD set the limits, and a reversed channel is fixed on the transmitter or with PIGPIO_INVERT. test_rc_template_wiring.py feeds MM1 serial lines and pigpio_rc pulse widths through complete.py's wiring with the shipped config and no custom map, and checks propagation, forward/reverse signs and the return to neutral. 1750, 1750 now gives steering −0.5 and throttle +0.5. |
Contributor
Author
|
I asked GPT Astra 6.1 for a code review and it found the following:
|
Found in review: a physical controller cannot drive a car built from complete.py, cv_control.py or path_follow.py. Reproduced by running complete.drive() with a scripted gamepad and a recording drivetrain: with the throttle stick held forward, the drivetrain received 0.0 on every pass. There are two causes, and this commit fixes the first one needed. UserSteering and UserThrottle ran only on the pass where the stick moved, because the templates used the axis event as their run condition. On every other pass something else decided user/steering and user/throttle -- in these templates the web controller, which writes 0.0 on every pass. So a stick held steady steered for one pass and then let go. Legacy's joystick part answered on every pass, which is why it never had this problem. The driving parts now run on every pass and answer with the last position they saw. Until the control has reported anything they answer None, which the vehicle loop does not write, so the web page drives until a controller is actually in use; after that the controller owns steering and throttle, as legacy had it. The position is held rather than the throttle, so a limit changed while the stick is held applies at once. TriggerThrottle already ran every pass on the trigger states, and now also answers None until a trigger has reported. On its own this changes nothing a car does: these parts still run after the drivetrain, so a held stick now survives every pass in memory and still never reaches the wheels. Moving them is the next commit. Phase 9 of the plan is added in this commit, since this is its first task. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Found in review: complete.py added the controller's parts after DriveMode, the drivetrain and the tub. Their steering and throttle landed in memory after the drivetrain had already run, and the next pass began with the web controller writing 0.0 over them -- so no physical controller drove a car built from complete.py, cv_control.py or path_follow.py, and the emergency stop's braking never reached the wheels. The comment at the call site said they went last "so that the emergency stop and the chaos monkey override what the driver asked for"; the order within the group already does that. add_controller_behaviors is now two functions. add_driving_behaviors -- steering, throttle, pilot mode, recording toggle, throttle limits, constant throttle, chaos monkey, emergency stop, stop vehicle -- is called straight after add_user_controller in all three templates. add_record_behaviors -- erase records and show the record count, which need the tub and feed nothing downstream -- stays where the tub exists. Testing it found a regression from the previous commit. With a stick's position held between events, a stick still held forward drove the car off again the moment an emergency stop finished, and so did constant throttle. Legacy's stop left the throttle at zero until the stick moved and turned constant throttle off. UserThrottle now takes 'user/estop' and lets go of the stick while a stop is in progress, and the stop clears 'user/constant_throttle'. Every test passed while none of this worked, because they checked memory at the end of a pass rather than what the drivetrain received. drive_harness.py runs a template's real drive() with a scripted controller and a recording drivetrain. test_drive_wiring.py uses it to check that a held stick reaches the drivetrain on every pass, that centring returns to neutral, that the web page drives until the controller reports, and that the braking sequence arrives and the car stays stopped afterwards. Moving the call back, dropping the estop input, or dropping the constant-throttle reset each fails it. path_follow.py imports distutils and cannot be imported on Python 3.12+, so its order is checked in the source. The RC test from the MM1 fix moved onto the same harness. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Contributor
Author
@TCIII I just pushed a commit that I think will fix the issue. |
Found by Copilot review: a disconnect only stopped the polling thread. Reproduced with the drive harness -- stick forward, then the pad's poll() raises OSError, and the drivetrain kept receiving +0.5 indefinitely. "Hold a stick's position between its events" made this dangerous: the lost pad's last throttle is now held for ever, where before the web controller's 0.0 won the next pass by accident. The PR's hardware test plan, T5.5, expects the loop to stop. Ed chose stopping the vehicle over a failsafe that keeps the loop running. InputControllerEvents now publishes CONTROLLER_LOST, a one-shot event, when its device fails -- an OSError from poll(), or any error that ends polling -- and not on an ordinary shutdown. add_driving_behaviors binds it, last in the group so it overrides everything above: on that pass the user throttle and steering go to 0.0 and the mode to user, so the drivetrain's final command is neutral whatever was driving, the pilot included, and StopVehicle ends the loop. Neutral is sent explicitly because some drivetrains do nothing on shutdown. Tested at both levels: the event pump publishes the loss on a disconnect or an unexpected error and not on a shutdown, and through complete.drive() a pad lost mid-acceleration gives the drivetrain one neutral pass and the loop ends, in autopilot too. Removing the binding or the event fails them. The drive harness now stops stepping once the vehicle turns itself off, as the real loop does. The migration guide says so: on main the loss went unnoticed, and a Bluetooth pad that sleeps when idle will now end a session. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The PR's hardware test plan, T4.5, expects turning constant throttle off to give "zero, not a lurch to whatever the stick reads". Since "Hold a stick's position between its events" it did not: with the stick held at 60%, turning constant throttle off gave 0.0 for one pass and then 0.3. Legacy set the throttle to zero until the stick moved. It is the same flaw the emergency stop had, and the lost controller: the held position outlived the thing that had taken the throttle away from it. "Run the driving and safety behaviors before their consumers" fixed the emergency-stop case alone, with one input to UserThrottle. That input is now any number of let-go signals -- emergency stop in progress, constant throttle on, controller lost -- and while any is true UserThrottle forgets the held position, so afterwards the throttle stays at zero until the stick moves. Tested through complete.drive() with T4.5 as written, and with the stick driving again once it moves. Dropping the constant-throttle signal fails it. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Ed ran the drive checks on a real car at 160aa3d: a fresh complete car with its wheels off the ground, an Xbox pad over Bluetooth, on Debian trixie. A held stick drives, the emergency stop brakes and the car stays stopped with the stick still held, constant throttle off gives zero (T4.5), both triggers end the loop, and switching the pad off logs the disconnect, stops the motor and ends the loop (T5.5). All passed. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Contributor
Author
Both confirmed and fixed, and it was worse than ordering alone: no physical controller could drive
New tests run each template's real Items 3 ( |
Item 3 of the review Ed posted on the PR: in basic.py's joystick branch, pressing the pilot-mode and recording buttons did nothing. Reproduced on the car through basic.drive() with a scripted Xbox pad -- after pressing A and B, user/mode stayed 'user' and recording stayed False. The branch adds a LocalWebController after TogglePilotMode and ToggleRecording and gave it only the camera image. So on every pass it wrote its own mode and recording, undoing both toggles in the same pass they happened. Legacy basic.py had no web controller here -- the gamepad was the controller -- so this came in when the template moved to behaviors. It is now given the current mode and recording as inputs, as complete.py gives it, so it adopts a value that changed rather than writing its own over it, and the web page shows what the gamepad set. test_basic_joystick.py drives basic.drive() with a scripted pad and checks that A cycles the mode and B toggles recording, and that each sticks. basic.py imports albumentations, so the test skips without it; on the car it fails against the unfixed template and passes with this change. Item 4, the simulator starting with no mode, is recorded in the plan as an open decision: some users may depend on it. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Found while Ed tried the basic.py toggle fix on hardware: a new basic car fails at startup because cfg_basic.py leaves PCA9685_I2C_BUSNUM as None and the deprecated Adafruit library cannot detect the bus on a Pi 4 on Debian trixie. It is not caused by this branch; the drivetrain code is the same on main. Behind it is a larger defect: cfg_basic.py advertises seven drivetrain types and basic.py honours only MOCK, sending every other value to the PCA9685 servo/ESC path. The plan now records the error, the workaround, and three options -- default the bus to 1 (recommended, a one-line PR to main), a fallback in the shared PCA9685 part (not recommended), and giving basic.py complete.py's add_drivetrain (an open decision). They are listed as proposed follow-ups, outside the task count, since none is agreed yet. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Ed: basic is not an important template, and the templates may be thinned out in a separate PR, which could remove basic.py rather than fix it. The findings and options stay in the plan for that PR; the two proposed items are marked postponed. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Contributor
T1.3 The pad is not reported as something else:
|
A third-party PS3 pad paired over Bluetooth on the car was measured with each control moved in isolation: the right stick's vertical axis, the left stick's horizontal axis and both triggers report the codes the map gives them, and the triggers rest at -1.0. The buttons were checked by hand. The other stick axes and the dpad were not measured, and it was not a genuine Sony pad, so the map is partly verified rather than verified. Also records why the same pad read through the 'xbox' map drives with the right trigger, which is how the mismatch showed up on the car. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Found on hardware: with the wheels off the ground and the throttle at zero, the emergency stop spun the motor on and off. It sent legacy's sequence -- full reverse at the throttle limit, a blip, full reverse again -- whatever the car was doing, and that car's ESC drives reverse directly rather than braking. EmergencyStop now keeps the recent forward peak of the throttle it is given, fading it to nothing over coast_time seconds (1.0 by default) once the throttle drops, since a fast car keeps rolling after it is released. A stop brakes at that level; with nothing to brake, or a car driving backwards, the throttle goes straight to zero. Braking from full throttle gives exactly legacy's sequence. The brake no longer depends on the throttle limit, so default_scale and the throttle-scale input are gone, and complete.py's call drops them too. It takes the drivetrain's last throttle as a new last input, so the pilot counts as well as the stick; complete.py passes it in the next commit. Phase 12 of the plan is added in this commit, since this is its first task. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
complete.py now passes 'throttle' -- what DriveMode gave the drivetrain on the previous pass -- so the stop brakes for the pilot and constant throttle as well as the stick. Under the pilot the user's throttle is zero while the car drives, so without it a stop in autopilot would not brake at all. cv_control.py and path_follow.py get this through add_driving_behaviors. ESTOP_COAST_TIME sets how long the brake takes to fade once the throttle is released. It is in cfg_complete.py, and the template reads it with a default so older car directories still start. Tests through complete.drive(): at a standstill the drivetrain never gets a negative throttle; a stick at 60% brakes at 60% of the limit; under the pilot it brakes at the pilot's throttle, which fails without the new input. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
All four checks passed on the car with the Xbox pad: no movement on an emergency stop at zero throttle or after the car has coasted, a short reverse kick from half throttle, and a weaker one just after release. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
The first of three steps to printing, once a pad is found, which device is active and what each of its controls does (Phase 13). description names the device; has_control answers for a control name, or None when the device cannot know in advance. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
BehaviorEventMapper.show_map now names the controllers it is given and prints a control | action | behavior table, as legacy's print_controls did, warning about any bound control none of them has. InputControllerEvents takes an on_ready hook, called once the device is found; a failing report is logged rather than losing the controller. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Every template that builds a BehaviorEventMapper now hands its show_map to the controller's InputControllerEvents as on_ready, instead of printing during assembly, where the map came before the pad was found and was buried by the start-up logs. complete.py shows the pad together with the web buttons, once; with only the web page, the web buttons' pump shows it. add_behavior_mapping is folded into add_user_controller. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Contributor
|
Tested revision a5566aa on a Raspberry Pi using MM1 for both RC input (CONTROLLER_TYPE = 'MM1') and drivetrain (DRIVE_TRAIN_TYPE = 'MM1'), via /dev/serial0 -> /dev/ttyS0. The tester ran the prepared bench-test application and reported that it worked and the PR looks good for this setup. The supplied checklist covered steering, forward/reverse throttle, simultaneous steering and throttle, neutral, and Ctrl-C shutdown; individual checklist results were not recorded separately. Software validation: 279 existing tests passed across RoboHAT input, controller factory, input events, behavior mapping, driving, safety, recording, and throttle limits. The fresh complete template imports successfully. Test output endpoints were MM1_MAX_FORWARD = 1600, MM1_STOPPED_PWM = 1500, MM1_MAX_REVERSE = 1400; automatic recording was disabled. One documentation observation: get_control_scaling() in this revision intentionally bypasses JOYSTICK_MAX_THROTTLE for MM1/RC receivers. The migration guide section 6 currently suggests JOYSTICK_MAX_THROTTLE to limit driver-requested throttle, which does not apply to MM1 in this revision. We used the MM1 output PWM endpoints instead. Scope: positive MM1 bench-test result, not verification of other controllers, RC signal-loss failsafe, recording/training, or floor driving. |
Moving a stick printed nothing, so a car that ignored its pad gave no clue whether the events arrived at all. With DONKEY_CONTROLLER_DEBUG=1 (or true/yes/on), InputControllerEvents logs every axis move, press, release, click, hold and the controller being lost, at INFO on its own logger, donkeycar.parts.controls.trace. Off by default; read once at construction. Adds Phase 14 to the plan and a "Seeing what the controller does while driving" section to CONTROLLER_MIGRATION.md. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
With DONKEY_CONTROLLER_DEBUG set, UserSteering, UserThrottle and TriggerThrottle log their output when it changes (rounded to three places, so a held stick logs once), with the axis and scale that produced it. UserThrottle logs letting go of the stick once per let-go. Together with 14.1's pad trace, a car that ignores its pad can be pinned to the pad, the map, or something downstream of the controller. CONTROLLER_MIGRATION.md explains how to read the two layers together. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Ed ran the controller trace on the car at 94f9c75 with a PS3 pad from ~/mycar1249, ~/mycar-cv-control-1249 and ~/mycar-path-follow-1249; both layers logged as designed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Finishes the controller rewrite prototyped in #1180, and closes out the design discussed in #1097.
A controller now reports which of its controls moved and nothing else.
InputControllerEventsturns that into one-shot events in vehicle memory,BehaviorEventMappertranslates those into named behaviors, and ordinary parts bind to the behaviors. So which button does what is a line inmyconfig.pyrather than an edit to a template — which was the point.donkeycar/parts/controller.pygoes from 1750 lines to 33.Important
Users upgrading a car should read
CONTROLLER_MIGRATION.md. It is ordered by "does your car still start", and covers every way an existing configuration is affected. Most cars need one or two lines changed.How to review this
47 commits, one per unit of work, each with tests passing and
mypy --strictclean. The two documents worth reading first:CONTROLLER_EVENTS_PLAN.md— the plan, with every task ticked and what it actually found. Written before the code and kept in step with it.CONTROLLER_MIGRATION.md— what breaks for users and what to do about it.Phases 0–2 are additive and independently mergeable; 5 and 6 must land together.
Two behaviour changes to look at deliberately
Everything else preserves existing behaviour. These do not:
JOYSTICK_DEADZONEnow applies to steering and throttle, not only to the record decision. Default0.01. Note that is smaller than the jitter measured on a real pad (0.048–0.099), so a car with worn sticks wants it raised.dead_zone=0.0restores the old behaviour exactly.Defects fixed that are live on
mainFound while porting, each with a regression test:
right_stick_horz/right_stick_vertnamed the codes the driver uses for the triggers; the real right stick had no name. Measured on a real pad — see the capture in the plan.right_trigger/left_trigger, names resolving to codes an Xbox pad never sends. It has never once run.L3/R3unbindable0x13awritten twice, as bothL3andshare. Python keeps the last, so both stick presses vanished silently.0x224, which is not a dpad code.hmmandwhatABS_HAT0X/ABS_HAT0Y.close()waited forever. Found on a real car; a car with an idle gamepad hung on shutdown.except:it replaced. Found on a real car.RCReceiver.shutdown()raised every timeChannelobject. pigpio was never released.basic.pywithUSE_RC = TrueraisedTypeErrorat startuppid.Kp -= deltaunbounded; a negative gain inverts the controller, so the car steers away from the line.CONTROLLER_TYPE = 'mock'raisedImportErrorMockControllerfrom a module that never defined one.AUTO_RECORD_ON_THROTTLEwas on, it logged a line and ignored the press.Verified on hardware
An Xbox One S pad on a Raspberry Pi 5 (Trixie, Python 3.13), throughout. That is where the two most serious defects above were found — neither is reachable through a test double, because a fake device never blocks and never disappears.
The Xbox control map is measured, not inferred: each control moved in isolation with the axis events timestamped. The other ten maps could not be verified — no hardware — and each says so in its own docstring rather than implying more confidence than it has.
Two decisions recorded rather than guessed
Both were open questions in the code, both settled by @Ezward, both pinned by tests so a later reader does not "fix" them:
MM1_STOPPED_PWM/MM1_MAX_FORWARD/MM1_MAX_REVERSElimit the output only. The input path appeared to use them but mapped throttle through the configured range and straight back out, which cancels exactly — so they have never affected input throttle however they were set.PS3JoystickandPS3JoystickPCstay separate. Merging would name axes a given driver does not report, which is precisely how Forza mode became dead code.Tests
707 passing locally, 706 on the car,
mypy --strictclean across 18 modules. Strictness is opted into per module viapyproject.toml, so it can spread without a repo-wide flag day.The gamepad maps get a check worth calling out: every default behavior map is verified against the controller it is for, and fails if a binding names a control the pad does not have. That is the Forza-mode defect class, now impossible to ship.
test_controller.pywas rewritten because it imported two deleted classes. What it did was call methods in sequence and assert nothing — it passed as long as nothing raised, and was skipped entirely without a joystick attached.Closes #1097. Supersedes the prototype in #1180, which can stay as the design record.
🤖 Generated with Claude Code
Test plan — on a real car
The unit tests cover the logic. This covers what only hardware shows: enumeration, timing, disconnection, shutdown, and whether the car actually drives. Two of the worst defects this PR fixes were found this way and are not reachable through a test double — a fake device never blocks and never disappears.
Tick one of Pass, Fail or N/A under each test. N/A is for a test you cannot run — no such pad, no such hardware — and it is worth saying which. Please record which controller and platform you used, since ten of the eleven control maps are unverified and this is how they get verified.
Caution
Put the car on blocks with its wheels off the ground for groups 1–5. Several tests deliberately apply full throttle and full reverse. Only group 6 needs the car on the floor.
Tester: _______________ Controller: _______________ Platform: _______________ Date: __________
Prerequisites
Group 1 — The controller is recognised
T1.0 Create a car that uses a game controller
T1.1 Every control gets a real name
Start the car. Read the control map it prints.
Expect: every axis and button named. Anything reporting as
axis(0x..)orbutton(0x...)is a control this PR's map does not cover — that is a finding, please paste the line.T1.2 The behavior map prints and reads correctly
Expect: a
Controller behaviors:table listing each behavior and the control bound to it. Every control named there is one your pad actually has.T1.3 The pad is not reported as something else
Expect: the device name matches your controller. If you have a PS3 pad and the controls are all wrong, try another
CONTROLLER_TYPEvariant — they differ by driver, not by pad.Group 2 — Events and timing
Run the event monitor rather than the car for this group. It runs the same
event pump a car does, at the same rate, so the timing you see is the timing
a behavior gets:
T2.1 Press and release
Press and release one button.
Expect:
/event/button/NAME/pressthen/event/button/NAME/release.T2.2 A single click fires
click/1— and only after the window closesClick once, then wait a second.
Expect:
click/1appears ~0.2s after the release, not instantly.T2.3 A double-click fires
click/2and notclick/1Click twice quickly.
Expect:
click/2only. Ifclick/1also appears, decision D1 has regressed — every single-click behavior would misfire partway through every double-click.T2.4 Triple and beyond
Click three times quickly, then four.
Expect:
click/3, thenclick/4. No lower counts alongside.T2.5 A long press fires
holdand suppresses the clickHold a button for over a second, then release.
Expect:
holdabout 0.5s after the press, and no click event on release. A long press is its own gesture.T2.6 Resting sticks are quiet
Touch nothing for 30 seconds.
Expect: few or no axis events. A steady trickle means
JOYSTICK_AXIS_EPSILONwants raising for your pad.Group 3 — Driving (wheels off the ground)
T3.1 Steering follows the stick
Expect: left moves the wheels left, right moves them right, centre returns to centre.
T3.2 Throttle follows the stick, in the right direction
Expect: pushing forward drives forward. If it is inverted,
JOYSTICK_THROTTLE_DIRis the setting — note whether the default was wrong for your pad.T3.3 The car does not drift with nobody touching it
Let go of everything for 30 seconds.
Expect: wheels still, throttle at zero. This is the dead-zone change. If it creeps, raise
JOYSTICK_DEADZONE— measured jitter on the test pad reached0.099against a default of0.01.T3.4 Xbox and Logitech only — the right stick is the right stick
Move the right stick vertically.
Expect: throttle responds. On
mainthis was really the right trigger — the axis names were wrong. Then squeeze each trigger: expect no throttle from them (unless you have bound Forza mode).T3.5 Steering and throttle together
Turn and accelerate at once.
Expect: both respond. Losing one is the "simultaneous changes dropped" defect class.
Group 4 — Behaviors
T4.1 Pilot mode steps round
Press the mode button three times.
Expect:
user→local_angle→local→user, each logged.T4.2 Recording follows the throttle
With
AUTO_RECORD_ON_THROTTLE = True, open the throttle then close it.Expect: recording starts and stops; tub count rises only while driving.
T4.3 Manual recording toggles
Set
AUTO_RECORD_ON_THROTTLE = False, restart, press the record button twice.Expect: recording on, then off. On
mainthis button silently did nothing whenever auto-record was on.T4.4 Throttle limit moves, and is felt
Press decrease several times, then drive. Press increase several times, then drive.
Expect: the logged scale changes, and the car's top speed changes with it.
T4.5 Constant throttle holds, and releases cleanly
Toggle it on, then off.
Expect: throttle holds at the current limit; turning it off gives zero, not a lurch to whatever the stick reads.
T4.6 Chaos monkey
Hold each shoulder button.
Expect: steering pulls to one side while held, returns on release.
T4.7 Erase records
Record some data, note the count, trigger erase.
Expect: the count drops by
ERASE_LAST_N_RECORDS, and the car keeps driving.Group 5 — Safety
Warning
Wheels off the ground. T5.1 applies full reverse deliberately.
T5.1 Emergency stop runs its whole sequence
Drive at moderate throttle, then trigger the e-stop.
Expect: the car brakes hard and comes to rest at zero throttle, not sitting in reverse. The sequence is reverse → brief neutral → reverse → wind back up to zero, over about a second. Mode should drop to
userand recording should stop.T5.2 Pressing e-stop repeatedly does not restart it
Trigger it and keep pressing.
Expect: one sequence that completes. Restarting on each press would never finish.
T5.3 The stop gesture needs both hands
Trigger the stop button without holding the modifier, then with it.
Expect: nothing the first time, clean exit the second. Stopping the car should be hard to do by accident.
T5.4 Shutdown is clean with an idle pad
Start the car, touch nothing for 30 seconds, then Ctrl-C.
Expect: it exits within a second or two. On
mainthis hung — the reader held a lock while blocked on the idle pad andclose()waited on it forever.T5.5 Unplugging the controller is reported
While driving, unplug the pad or switch it off.
Expect: a log line saying the controller disconnected, and the loop stops. On
mainthis was silent — the pad vanished and every input went nowhere with nothing in the log.Group 6 — On the floor
T6.1 Drive a lap manually
Expect: steering and throttle feel like they did before this PR. Any change in feel is a finding — the only intended change is the dead zone.
T6.2 Record a tub and train on it
Expect: data records, trains, and the resulting pilot drives. Confirms nothing upstream of the tub changed shape.
T6.3 E-stop at speed
On the floor, at real speed, trigger the e-stop.
Expect: the car stops promptly and does not reverse away afterwards.
Group 7 — Configuration and migration
T7.1 An existing car config still starts
Take a car directory from before this PR, point it at this branch, run it.
Expect: it starts. If it does not, the error should be one the migration guide names. Anything else is a gap in the guide — please say which.
T7.2 Remapping a behavior works without touching a template
Set
CONTROLLER_BEHAVIOR_MAPinmyconfig.pyto move one behavior to a different button. Restart.Expect: the new button does it, the old one does not. This is the whole point of the PR.
T7.3 One behavior, two controls
Bind a behavior to a gamepad button and a web button as a list.
Expect: either triggers it. This replaces the branching every template used to carry.
T7.4 Web buttons produce real events
Press
web/w1–w5in the web UI.Expect: they behave like gamepad buttons, including double-click.
T7.5 Renaming one control
Set
JOYSTICK_BUTTON_NAMES = {0x13c: 'guide'}(or a code your pad reports) and restart.Expect: that control reports under the new name; everything else is unchanged.
T7.6
donkey createjsExpect: it asks you to press controls, names them, and prints
JOYSTICK_BUTTON_NAMES/JOYSTICK_AXIS_NAMES. Paste the output intomyconfig.pywithCONTROLLER_TYPE = 'custom'and confirm the names take.T7.7 The
donkeyCLI is intactExpect: every command listed, including
createjs. This is the import chain that forced 6.1 and 6.2 to swap order.Please also record
Ten of the eleven control maps in this PR are not verified against hardware — only the Xbox One S is. If you ran group 1 on any other pad, paste its control map into a comment. That turns an inferred map into a measured one, and it is the single most useful thing a tester can contribute here.