diff --git a/HARDWARE_TESTING.md b/HARDWARE_TESTING.md index 6250eb0d..1468d495 100644 --- a/HARDWARE_TESTING.md +++ b/HARDWARE_TESTING.md @@ -2131,9 +2131,11 @@ app touches first takes that path for the other two. ## 22. Guide button LED brightness The intensity the driver sends at `startDevice()` is now the user's setting rather than a -hardcoded `0x14`, mapped from four presets in `PreferenceConfiguration.getGuideButtonLedValue()`: +hardcoded `0x14`, mapped from the presets in `PreferenceConfiguration.getGuideButtonLedValue()`: off `0x00`, dim `0x0A`, normal `0x14`, bright `0x2F`. `normal` is what the driver sent before, so -an untouched install behaves exactly as it did. +an untouched install behaves exactly as it did. A fifth setting, "Battery level", starts at +`normal` and then moves the LED with each battery report — that is section 31's to verify; this +section covers the four fixed presets. Two of those values are assertions about hardware that nothing off a device can settle. @@ -2176,8 +2178,9 @@ next created — the next stream — not on re-plugging a pad. That is the exist constructor path is wired up, not just the adapter's. - [ ] **Both ABIs.** The change is in the shared driver, but the Shield is `arm64-v8a` and the Homatics `armeabi-v7a`, and only a run on each proves the JNI signature change - (`createDriver` is now `(II)J`) binds. A mismatch fails at library load, so the symptom would - be the adapter not starting at all rather than the LED misbehaving. + (`createDriver` is now `(II)J`, and since section 31 `setLedNative` is `(JII)Z`) binds. A + mismatch fails at library load, so the symptom would be the adapter not starting at all + rather than the LED misbehaving. --- @@ -2771,6 +2774,93 @@ it would say whether "never drop" was ever more than an accident of the PTS base --- +## 31. Controller battery: the in-game label, and the guide LED following it + +Two additions built on the battery reports the app already collected and only ever forwarded to +the host. Neither touches a per-frame path: battery writes happen at battery-change frequency on +driver threads, the label is built once per menu level while the decoder is already stalled behind +the dialog, and the LED sends one GIP frame per *level* change on the driver's read thread. + +**The label.** While any level of the in-game menu is up, a small end-aligned line along the +bottom of the screen lists every controller's battery, one entry per host player number: +`Controller 1: 75% · Controller 2: 50%, charging · Controller 3: wired`. "wired" is a GIP pad +running off its cable with no battery pack — it reports `NOT_CHARGING` and no percentage, and the +cable is the only fact in that. The store behind it is `ControllerBatteries`; the menu keeps it +shown across submenus with a `DialogChain` token and hides it when its last dialog closes. + +The GIP percentages are the spec's four levels mapped as `GipController.updateBattery()` always +has: full 100, medium 50, low 25, critical 10. Shield and Bluetooth pads report real percentages. + +Also fixed on the way: a GIP pad's first status packet usually arrives before its first input +report, which is what assigns its player number, so that first battery report used to reach the +host as player 0. It is now held on the context until the number is assigned. And on the cabled +path, a status that landed before the Java object existed used to arm the driver's change filter +with nobody listening, so Java heard nothing until the battery actually moved; the filter now +stays unarmed until a report is delivered, and MS-GIPUSB 2.2.9 has the pad repeat status every +second for its first ten seconds. + +**The LED.** "Battery level" is a fifth value of the Xbox button brightness setting. The guide LED +is monochrome, so following the battery means brightness steps, taken from the presets so each one +is a level the user has already seen: full → bright `0x2F`, medium → normal `0x14`, low → dim +`0x0A`, and critical → a **slow blink** (`LED_BLINK_SLOW`, `0x04`) at normal intensity, because a +warning has to look like one and a dimmer steady LED across a room does not. A pad with no battery +fitted (type 0, running off its cable) goes bright. Before the first report, and for any level the +spec does not define, the pad shows the normal start value. The ladder is +`GuideButtonLed.forBattery()`, on the Java side so it is tested; the native side only applies what +it is handed and refuses a pattern outside `enum LedMode`. + +Like section 22, the setting is read once in `UsbDriverService.onCreate()`, so the procedure is +"change setting, leave the stream, start it again". + +The open hardware questions: + +- [ ] **The label is legible under the dim.** Open the menu with two pads in different states. + The dialog dims everything behind it at the theme's default 60%; the text is the primary + colour rather than the secondary one the settings version label uses for exactly this + reason. If it is still too faint from the sofa, that is a colour change, not a dim change. +- [ ] **Several controllers, in player order.** Both pads listed, numbered as the host numbers + them. The pad-audio submenu now uses the same numbers (it used to count pads by their + position in the driver's list, which disagreed whenever the Shield controller or a + Bluetooth pad held a slot): with the Shield controller as player 1, a GIP pad should read + "Controller 2" in both places. A paired pad nobody has touched reads "Unused controller" in + the audio menu until it sends input, and is absent from the label. With multi-controller + off the audio menu keeps positional numbers, since every pad is player 0 there. +- [ ] **The label survives the menu's own navigation.** Open Controllers, then Back: the label + stays. Cancel, or Back at the root: the label goes. Choose Disconnect: the label goes. +- [ ] **A pad that unplugs mid-menu is gone on the next open.** Its number is released, which + clears its slot. +- [ ] **A battery-less cabled pad reads "wired".** Requires a pad with the pack removed, on the + wired GIP path (`wiredPadAudio` on). +- [ ] **The LED steps down over a discharge.** With "Battery level" set, run a pad down: bright at + full, visibly dimmer at medium, dimmer again at low. Each step is one log line: + ```bash + adb shell setprop persist.log.tag '""' + adb logcat -d | grep -a "Guide button LED set to" + adb shell setprop persist.log.tag S + ``` + One line per level change, and none for a charge-state-only change (plug and unplug the + cable at a steady level): `GipController` deduplicates against the last command sent. +- [ ] **Critical actually blinks.** Table 42 marks every pattern except On as "not implemented by + host" — which describes Microsoft's host, not the pad — so whether a pad honours + `LED_BLINK_SLOW` is unverified, and xone's exposing the patterns through sysfs is inference + rather than proof. If the pad sits steady at `0x14` instead, or logs `Refused`/no ack, try + `BLINK_MED`, `BLINK_FAST` and the two `FADE`s in turn and record which of them reads as a + warning; only if none does, fall back to `PATTERN_ON` at `0x04` and note that critical then + reads like "dim". The spec also has hosts wait for two consecutive critical reports before + warning the user; the LED reacts to the first. +- [ ] **A battery-less cabled pad goes bright.** Type 0 in the status byte, whatever the level + field says (the spec has such a pad report critical in the level bits). +- [ ] **A pad pairing mid-stream picks the ladder up.** Its first status after `startDevice()` + drives it; nothing about the setting is per-pad. +- [ ] **The cabled pad, wired GIP path.** Same ladder through `WiredController`, plus the + first-status fix above: the label and the LED should both reflect the pad within a couple of + seconds of it being claimed, not only after the battery next moves. +- [ ] **Both ABIs load.** `setLedNative` is a new `(JII)Z` entry in the controller's JNI table; a + mismatch fails at library load, so the symptom is the adapter or cabled pad not starting + at all rather than the LED misbehaving. + +--- + ## Hardware still needed | Needed for | Hardware | @@ -2798,6 +2888,7 @@ it would say whether "never drop" was ever more than an accident of the PTS base | §17.1 sub-50 fractional | A 23.976 or 29.97 Hz output mode specifically — 59.94 takes the other branch and cannot settle it | | §18.2 two-client check | A second Moonlight client against the same host | | §30 debug boxes | Shield: a debug build in min-latency for the presented counter, then cap-FPS for the gating check. Homatics: any build in min-latency. The mechanism itself is verified on the Shield's release build | +| §31 ladder | A rechargeable adapter pad run down to critical, a play-and-charge cable for the charge-only case, and a cabled pad with its battery pack removed for the "wired" and type-0 cases | | §19 | Both target devices; the survey differs per SoC | | §26 badge centring | Either box, on a television at normal viewing distance | | §20 | The Homatics specifically — the Shield cannot verify a change scoped to `armeabi-v7a` | diff --git a/app/src/main/java/com/limelight/Game.java b/app/src/main/java/com/limelight/Game.java index e6df26c0..50a4f793 100644 --- a/app/src/main/java/com/limelight/Game.java +++ b/app/src/main/java/com/limelight/Game.java @@ -4,6 +4,7 @@ import com.limelight.binding.PlatformBinding; import com.limelight.binding.audio.LowLatencyAudioRenderer; import com.limelight.binding.audio.PadAudioSink; +import com.limelight.binding.input.ControllerBatteries; import com.limelight.binding.input.ControllerHandler; import com.limelight.binding.input.GameInputDevice; import com.limelight.binding.input.KeyboardTranslator; @@ -89,6 +90,7 @@ import java.security.cert.CertificateFactory; import java.security.cert.X509Certificate; import java.util.ArrayDeque; +import java.util.ArrayList; import java.util.Collections; import java.util.List; import java.util.Locale; @@ -166,6 +168,10 @@ private record ScheduledStep(long uptimeMs, TextKeyPlanner.Step step) {} // comment. private TextView imePreviewOverlay; + // The controllers' battery levels, along the bottom edge while the in-game menu is up. Never + // focused either; the menu's dialog owns focus for as long as this is visible. + private TextView controllerBatteryOverlay; + // Bottom margin currently applied to the echo, so the insets listener only re-lays it out when // the keyboard actually changes size rather than on every insets change during a stream. private int imePreviewInset = -1; @@ -446,6 +452,7 @@ protected void onCreate(Bundle savedInstanceState) { performanceSparklines = findViewById(R.id.performanceSparklines); imePreviewOverlay = findViewById(R.id.imePreviewOverlay); + controllerBatteryOverlay = findViewById(R.id.controllerBatteryOverlay); inputCaptureProvider = InputCaptureManager.getInputCaptureProvider(this); @@ -2470,7 +2477,8 @@ public void togglePerformanceOverlay() { } /** - * Opens the in-stream menu. + * Opens the in-stream menu. The menu also shows every controller's battery along the bottom + * of the screen for as long as it is up - see {@link #showControllerBatteryLabel}. * * @param device the controller that requested it, so the menu can offer that controller's own * options, or null when opened from a keyboard or gesture @@ -2485,6 +2493,49 @@ public void showGameMenu(GameInputDevice device) { new GameMenu(this, conn, device); } + /** + * Fills the battery label from the controllers' last readings and shows it, or hides it when + * no controller has anything to show. One entry per controller, in player order, so several + * pads read side by side rather than the last one reported winning. + * + *

Numbered as the host numbers them - player 1 is controller number 0 - which is also how + * the pad-audio submenu names pads, so a number here is the same pad there. + * + *

UI thread only: the menu opens from {@link #onBackPressed} or a controller button on the + * main thread and calls this before showing each level. The label is a child of this + * activity's window, not a new layer, and the decoder is already stalled behind the menu's + * dialog, so the cost of building the text is not on any frame. + */ + public void showControllerBatteryLabel() { + var parts = new ArrayList(); + for (ControllerBatteries.Reading reading : controllerHandler.batterySnapshot()) { + int player = reading.controllerNumber() + 1; + switch (reading.display()) { + case LEVEL -> parts.add(getString(R.string.controller_battery_level, + player, reading.percentage())); + case CHARGING -> parts.add(getString(R.string.controller_battery_charging, + player, reading.percentage())); + case WIRED -> parts.add(getString(R.string.controller_battery_wired, player)); + // Never in a snapshot; listed so the switch stays exhaustive when Display grows + case HIDDEN -> { } + } + } + + if (parts.isEmpty()) { + controllerBatteryOverlay.setVisibility(View.GONE); + return; + } + + controllerBatteryOverlay.setText( + String.join(getString(R.string.controller_battery_separator), parts)); + controllerBatteryOverlay.setVisibility(View.VISIBLE); + } + + /** Takes the battery label down. UI thread; the menu calls it when its last dialog closes. */ + public void hideControllerBatteryLabel() { + controllerBatteryOverlay.setVisibility(View.GONE); + } + /** * @return true if the USB driver has an Xbox wireless adapter running, which is what decides * whether the game menu offers to pair a controller with it. False when the USB @@ -2520,6 +2571,11 @@ public PadAudioSink getPadAudioSink() { return padAudioSink; } + /** @return the multi-controller setting, which decides how the game menu numbers pads */ + public boolean isMultiControllerEnabled() { + return prefConfig.multiController; + } + /** * Turns audio to a pad's headphone jack on or off mid-stream, reporting the outcome. A refusal * is either the two-pad bandwidth limit or a pad with no audio endpoint at all, and the toast diff --git a/app/src/main/java/com/limelight/GameMenu.java b/app/src/main/java/com/limelight/GameMenu.java index 8c6a18e2..31931d26 100644 --- a/app/src/main/java/com/limelight/GameMenu.java +++ b/app/src/main/java/com/limelight/GameMenu.java @@ -9,15 +9,19 @@ import com.limelight.binding.input.driver.GipController; import com.limelight.nvstream.NvConnection; import com.limelight.nvstream.input.KeyboardPacket; +import com.limelight.utils.DialogChain; import com.limelight.utils.MenuDialog; import java.util.ArrayList; +import java.util.Comparator; import java.util.List; /** * Provide options for ongoing Game Stream. *

- * Shown on back action in game activity. + * Shown on back action in game activity. While any level of it is up, {@link Game} also shows the + * controllers' battery levels along the bottom of the screen; the menu is what decides when that + * label appears and goes away. */ public class GameMenu { @@ -41,6 +45,9 @@ public MenuOption(String label, Runnable runnable) { private final NvConnection conn; private final GameInputDevice device; + // One menu is one chain of dialogs; see showMenuDialog for what it settles. + private final DialogChain chain = new DialogChain(); + /** Building the menu shows it immediately; there is no separate show call. */ public GameMenu(Game game, NvConnection conn, GameInputDevice device) { this.game = game; @@ -129,6 +136,11 @@ private void showMenuDialog(String title, MenuOption[] options) { * own dialog with no relationship to the one that opened it, so without this a submenu's Back * dropped straight to the stream - further out than the user asked to go. The root passes * null, where dismissing really does mean returning to the game. + * + *

The battery label is shown afresh at every level, so a reading that changed while a + * submenu was up is picked up on the way back, and hidden only when the whole chain ends. + * Every dialog's {@code onDismiss} fires, including a parent's after its child has opened, so + * each one takes a {@link DialogChain} token and only the latest holder hides the label. */ private void showMenuDialog(String title, MenuOption[] options, Runnable onBack) { List rows = new ArrayList<>(); @@ -142,7 +154,14 @@ private void showMenuDialog(String title, MenuOption[] options, Runnable onBack) : runnable)); } - MenuDialog.show(game, title, 0, rows, onBack, null); + int token = chain.opened(); + game.showControllerBatteryLabel(); + + MenuDialog.show(game, title, 0, rows, onBack, () -> { + if (chain.closes(token)) { + game.hideControllerBatteryLabel(); + } + }); } /** @@ -152,14 +171,33 @@ private void showMenuDialog(String title, MenuOption[] options, Runnable onBack) * cannot be enabled because the two-pad limit is reached says so on the row. The cap is a * bandwidth budget on a link shared with controller input, so it is worth showing rather * than letting a selection quietly do nothing. + * + *

Pads are named by the host's player number - see {@link GameMenuLayout#padNumber} for + * why, and for the two cases that cannot be - and listed in that order, with any pad that + * has no number yet at the end. */ private void showPadAudioMenu() { List controllers = game.getGipControllers(); PadAudioSink sink = game.getPadAudioSink(); List options = new ArrayList<>(); + boolean multiController = game.isMultiControllerEnabled(); - int number = 1; - for (GipController controller : controllers) { + // Number before sorting: the position fallback is the driver's order, which is this one + var numbers = new int[controllers.size()]; + for (int i = 0; i < controllers.size(); i++) { + numbers[i] = GameMenuLayout.padNumber(multiController, + controllers.get(i).getPlayerNumber(), i + 1); + } + var order = new ArrayList(); + for (int i = 0; i < controllers.size(); i++) { + order.add(i); + } + order.sort(Comparator.comparingInt(i -> + numbers[i] == GameMenuLayout.PAD_UNNUMBERED ? Integer.MAX_VALUE : numbers[i])); + + for (int i : order) { + GipController controller = controllers.get(i); + int number = numbers[i]; boolean enabled = sink.isEnabled(controller); /* @@ -197,8 +235,9 @@ else if (sink.canEnableMore()) { state = getString(R.string.game_menu_pad_audio_unavailable); } - String label = game.getResources().getString( - R.string.game_menu_pad_audio_entry, number++, state); + String label = number == GameMenuLayout.PAD_UNNUMBERED + ? game.getResources().getString(R.string.game_menu_pad_audio_entry_unnumbered, state) + : game.getResources().getString(R.string.game_menu_pad_audio_entry, number, state); options.add(new MenuOption(label, () -> game.togglePadAudio(controller))); } diff --git a/app/src/main/java/com/limelight/GameMenuLayout.java b/app/src/main/java/com/limelight/GameMenuLayout.java index 4bcca225..d05504aa 100644 --- a/app/src/main/java/com/limelight/GameMenuLayout.java +++ b/app/src/main/java/com/limelight/GameMenuLayout.java @@ -115,4 +115,31 @@ public static boolean hasControllerOptions(State state) { private static boolean padAudioAvailable(State state) { return state.hasGipPads() && state.padAudioFormatSupported(); } + + /** {@link #padNumber} for a pad the host has not been told about yet. */ + public static final int PAD_UNNUMBERED = 0; + + /** + * The number a pad-audio row shows for a pad: the host's player number, so it is the number + * the game on the host uses for that pad, and the same one the battery label along the + * bottom of the screen uses. The rows used to count pads by their position in the driver's + * list, which disagreed with both whenever an Android-enumerated pad held a slot. + * + *

Two cases fall back. A pad is numbered on its first input report, not before, so a + * paired pad nobody has touched has no number yet and is reported as + * {@link #PAD_UNNUMBERED} for the row to say so - pressing anything on it numbers it. With + * multi-controller off every pad is player 0, so player numbers would make every row + * "Controller 1"; that mode keeps the positional count, which at least tells the rows apart. + * + * @param multiController the multi-controller setting + * @param playerNumber the pad's 0-based player number, or negative if it has none yet + * @param position the pad's 1-based position in the driver's list + * @return the 1-based number to show, or {@link #PAD_UNNUMBERED} + */ + public static int padNumber(boolean multiController, int playerNumber, int position) { + if (!multiController) { + return position; + } + return playerNumber < 0 ? PAD_UNNUMBERED : playerNumber + 1; + } } diff --git a/app/src/main/java/com/limelight/binding/input/ControllerBatteries.java b/app/src/main/java/com/limelight/binding/input/ControllerBatteries.java new file mode 100644 index 00000000..de33109e --- /dev/null +++ b/app/src/main/java/com/limelight/binding/input/ControllerBatteries.java @@ -0,0 +1,142 @@ +package com.limelight.binding.input; + +import com.limelight.nvstream.jni.MoonBridge; + +import java.util.ArrayList; +import java.util.List; + +/** + * The last battery reading of every attached controller, kept so the in-game menu can show them. + * + *

Split out of {@link ControllerHandler} for the same reason {@link ControllerLedColor} is: the + * handler needs a real {@code InputDevice} and cannot be loaded from a JVM test. Battery state used + * to be fire-and-forget to the host, so nothing on the box could show it; this is the one place it + * is retained, and the only consumer is the label the menu shows. + * + *

Slots are indexed by host player number rather than device id, because that is the number the + * user sees on the host and the one the label prints. The attached-controller mask is a + * {@code short}, which is where {@link #MAX_CONTROLLERS} comes from. + * + *

Written from two threads that never meet the reader: a GIP pad reports from its driver's read + * thread, an Android-enumerated pad from the battery {@code HandlerThread}, and the label reads on + * the UI thread once per menu level. {@code synchronized} is the right tool at those rates — a few + * writes per session, one read per menu open — and keeps the snapshot consistent without any + * cleverness. + * + *

References {@code MoonBridge.LI_BATTERY_*} freely: they are {@code static final byte} + * compile-time constants, so {@code javac} inlines them and this class never triggers + * {@code MoonBridge}'s static initialiser, which loads a native library and would kill a JVM test. + * Nothing else in {@code MoonBridge} may be touched from here. + * + *

Latency: none. Nothing here is on the frame path or the input path. + */ +public final class ControllerBatteries { + + /** One more than the highest player number; the attached-controller mask is 16 bits wide. */ + public static final int MAX_CONTROLLERS = 16; + + /** + * What a reading is worth showing as. {@link #HIDDEN} never leaves {@link #snapshot()}: it is + * the classifier's way of saying the reading carries nothing the user can act on. + */ + public enum Display { + /** A percentage, discharging or otherwise not being filled. */ + LEVEL, + /** A percentage, and the pad is charging. */ + CHARGING, + /** No battery level at all, but the pad is on a cable — a GIP pad with no battery pack. */ + WIRED, + /** Nothing to show. */ + HIDDEN + } + + /** + * One controller worth showing. + * + * @param controllerNumber host player number, 0-based + * @param display what to show it as; never {@link Display#HIDDEN} + * @param percentage 0 to 100; only meaningful for {@link Display#LEVEL} and + * {@link Display#CHARGING} + */ + public record Reading(int controllerNumber, Display display, int percentage) { + } + + // Moonlight's own units, exactly as they go to the host, so the label and the host never + // disagree about a pad. Percentage is 0 - 100 or LI_BATTERY_PERCENTAGE_UNKNOWN. + private final byte[] states = new byte[MAX_CONTROLLERS]; + private final byte[] percentages = new byte[MAX_CONTROLLERS]; + private final boolean[] present = new boolean[MAX_CONTROLLERS]; + + /** + * Records a reading, replacing whatever the slot held. + * + *

An out-of-range number is ignored rather than thrown on: this is fed from driver threads, + * and an exception there takes the pad's read loop down with it. + * + * @param controllerNumber host player number + * @param state one of {@code MoonBridge.LI_BATTERY_STATE_*} + * @param percentage 0 to 100, or {@code MoonBridge.LI_BATTERY_PERCENTAGE_UNKNOWN} + */ + public synchronized void update(int controllerNumber, byte state, byte percentage) { + if (controllerNumber < 0 || controllerNumber >= MAX_CONTROLLERS) { + return; + } + states[controllerNumber] = state; + percentages[controllerNumber] = percentage; + present[controllerNumber] = true; + } + + /** Forgets a slot, for a controller whose player number has been released. */ + public synchronized void clear(int controllerNumber) { + if (controllerNumber < 0 || controllerNumber >= MAX_CONTROLLERS) { + return; + } + present[controllerNumber] = false; + } + + /** + * @return every controller with something to show, in ascending player number. Allocates a + * small list per call, which is fine: it is called once per menu level, never per frame. + */ + public synchronized List snapshot() { + List readings = new ArrayList<>(); + for (int i = 0; i < MAX_CONTROLLERS; i++) { + if (!present[i]) { + continue; + } + Display display = classify(states[i], percentages[i]); + if (display == Display.HIDDEN) { + continue; + } + readings.add(new Reading(i, display, percentages[i] & 0xFF)); + } + return readings; + } + + /** + * Decides what a reading shows as. Package-private so the table can be tested directly. + * + *

The unknown-percentage cases are the non-obvious ones. A GIP pad running off its cable + * with no battery pack reports {@code NOT_CHARGING} with an unknown percentage, because the spec + * gives it no level to report; the only fact in that reading is that it is on a cable, which is + * still worth a word. {@code CHARGING} with no percentage is the same fact. Everything else + * without a percentage says nothing. + * + *

With a percentage, only {@code CHARGING} needs distinguishing: {@code DISCHARGING}, + * {@code FULL}, {@code NOT_CHARGING} and even {@code UNKNOWN} — which Android's + * {@code BatteryState} can report alongside a real capacity — all just show the level. + */ + static Display classify(byte state, byte percentage) { + if (state == MoonBridge.LI_BATTERY_STATE_NOT_PRESENT) { + return Display.HIDDEN; + } + if (percentage == MoonBridge.LI_BATTERY_PERCENTAGE_UNKNOWN) { + return switch (state) { + case MoonBridge.LI_BATTERY_STATE_CHARGING, + MoonBridge.LI_BATTERY_STATE_NOT_CHARGING -> Display.WIRED; + default -> Display.HIDDEN; + }; + } + return state == MoonBridge.LI_BATTERY_STATE_CHARGING ? Display.CHARGING : Display.LEVEL; + } +} diff --git a/app/src/main/java/com/limelight/binding/input/ControllerHandler.java b/app/src/main/java/com/limelight/binding/input/ControllerHandler.java index a4b57564..e47c560a 100644 --- a/app/src/main/java/com/limelight/binding/input/ControllerHandler.java +++ b/app/src/main/java/com/limelight/binding/input/ControllerHandler.java @@ -145,6 +145,11 @@ public class ControllerHandler implements InputManager.InputDeviceListener, UsbD private final SparseArray inputDeviceContexts = new SparseArray<>(); private final SparseArray usbDeviceContexts = new SparseArray<>(); + // What the in-game menu's battery label reads. Kept apart from the contexts above because + // those are touched from the USB driver thread (deviceAdded) and the label reads on the UI + // thread; iterating a SparseArray across that pair is a race, this store is not. + private final ControllerBatteries batteries = new ControllerBatteries(); + private final NvConnection conn; private final Activity activityContext; private final double stickDeadzone; @@ -448,6 +453,15 @@ private void releaseControllerNumber(GenericControllerContext context) { if (context.reservedControllerNumber) { LimeLog.info("Controller number "+context.controllerNumber+" is now available"); currentControllers &= ~(1 << context.controllerNumber); + + // Only a reserved number is a pad's own. With multi-controller off every pad shares + // number 0, the last reporter's battery is the one shown, and nothing clears it until + // the stream ends - accepted, since that mode merges the pads for the host too. + batteries.clear(context.controllerNumber); + } + + if (context instanceof UsbDeviceContext usbContext) { + usbContext.device.setPlayerNumber(AbstractController.NO_PLAYER_NUMBER); } // If this device sent data as a gamepad, zero the values before removing. @@ -603,6 +617,35 @@ else if (!devContext.hasJoystickAxes) { // Report attributes of this new controller to the host context.sendControllerArrival(); + + if (context instanceof UsbDeviceContext usbContext) { + // The game menu names a pad by this number, and holds only the controller + usbContext.device.setPlayerNumber(context.controllerNumber); + + // A GIP pad's first status packet usually lands before its first input report, which + // is what brought us here, so its battery was cached rather than sent - see + // reportControllerBattery. Now that the pad has a number, hand it on. + if (usbContext.hasBatteryReading) { + publishBattery(usbContext, usbContext.batteryState, usbContext.batteryPercentage); + } + } + } + + /** + * Reports a controller's battery to the host and to the in-game menu's label together, so the + * two never disagree about a pad. + */ + private void publishBattery(GenericControllerContext context, byte state, byte percentage) { + conn.sendControllerBatteryEvent((byte) context.controllerNumber, state, percentage); + batteries.update(context.controllerNumber, state, percentage); + } + + /** + * @return every controller with a battery reading worth showing, in player order. For the + * in-game menu's label; safe to call from the UI thread. + */ + public List batterySnapshot() { + return batteries.snapshot(); } /** @@ -1157,42 +1200,25 @@ else if (chargingState == SceChargingState.CHARGING) { if (currentBatteryStatus != context.lastReportedBatteryStatus || !areBatteryCapacitiesEqual(currentBatteryCapacity, context.lastReportedBatteryCapacity)) { - byte state; - byte percentage; - - switch (currentBatteryStatus) { - case BatteryState.STATUS_UNKNOWN: - state = MoonBridge.LI_BATTERY_STATE_UNKNOWN; - break; - - case BatteryState.STATUS_CHARGING: - state = MoonBridge.LI_BATTERY_STATE_CHARGING; - break; - - case BatteryState.STATUS_DISCHARGING: - state = MoonBridge.LI_BATTERY_STATE_DISCHARGING; - break; - - case BatteryState.STATUS_NOT_CHARGING: - state = MoonBridge.LI_BATTERY_STATE_NOT_CHARGING; - break; - - case BatteryState.STATUS_FULL: - state = MoonBridge.LI_BATTERY_STATE_FULL; - break; - - default: - return; - } - - if (Float.isNaN(currentBatteryCapacity)) { - percentage = MoonBridge.LI_BATTERY_PERCENTAGE_UNKNOWN; - } - else { - percentage = (byte)(currentBatteryCapacity * 100); - } - - conn.sendControllerBatteryEvent((byte)context.controllerNumber, state, percentage); + byte state = switch (currentBatteryStatus) { + case BatteryState.STATUS_UNKNOWN -> MoonBridge.LI_BATTERY_STATE_UNKNOWN; + case BatteryState.STATUS_CHARGING -> MoonBridge.LI_BATTERY_STATE_CHARGING; + case BatteryState.STATUS_DISCHARGING -> MoonBridge.LI_BATTERY_STATE_DISCHARGING; + case BatteryState.STATUS_NOT_CHARGING -> MoonBridge.LI_BATTERY_STATE_NOT_CHARGING; + case BatteryState.STATUS_FULL -> MoonBridge.LI_BATTERY_STATE_FULL; + // Not a status Moonlight has a word for; leave the last report standing. A + // switch expression cannot return from the method, hence the sentinel. + default -> (byte) -1; + }; + if (state < 0) { + return; + } + + byte percentage = Float.isNaN(currentBatteryCapacity) + ? MoonBridge.LI_BATTERY_PERCENTAGE_UNKNOWN + : (byte)(currentBatteryCapacity * 100); + + publishBattery(context, state, percentage); context.lastReportedBatteryStatus = currentBatteryStatus; context.lastReportedBatteryCapacity = currentBatteryCapacity; @@ -2929,19 +2955,33 @@ public void reportControllerMotion(int controllerId, byte motionType, float moti conn.sendControllerMotionEvent((byte) context.controllerNumber, motionType, motionX, motionY, motionZ); } - /** {@inheritDoc} Battery state from a controller driven by our own USB drivers. */ + /** + * {@inheritDoc} Battery state from a controller driven by our own USB drivers. + * + *

Held back until the pad has a player number. A USB pad is numbered on its first input + * report, and a GIP pad's first status packet usually arrives before that, so this used to + * reach the host as player 0 whatever slot the pad went on to take. The reading is cached on + * the context instead and sent by {@link #assignControllerNumberIfNeeded} once the number is + * known; anything after that goes straight through to the host and the menu's label. + */ @Override public void reportControllerBattery(int controllerId, byte batteryState, byte batteryPercentage) { if (stopped) { return; } - GenericControllerContext context = usbDeviceContexts.get(controllerId); + UsbDeviceContext context = usbDeviceContexts.get(controllerId); if (context == null) { return; } - conn.sendControllerBatteryEvent((byte) context.controllerNumber, batteryState, batteryPercentage); + context.batteryState = batteryState; + context.batteryPercentage = batteryPercentage; + context.hasBatteryReading = true; + + if (context.assignedControllerNumber) { + publishBattery(context, batteryState, batteryPercentage); + } } /** {@inheritDoc} Releases the context and player number for a USB controller that went away. */ @@ -3472,6 +3512,14 @@ public void enableSensors() { class UsbDeviceContext extends GenericControllerContext { public AbstractController device; + // The last battery the driver reported, in Moonlight's units, kept until the pad has a + // player number to report it under. Written by reportControllerBattery and read by + // assignControllerNumberIfNeeded, both on this device's own driver thread - the same + // thread reportControllerState arrives on - so no volatile, and no lock. + public byte batteryState; + public byte batteryPercentage; + public boolean hasBatteryReading; + @Override public void destroy() { super.destroy(); diff --git a/app/src/main/java/com/limelight/binding/input/driver/AbstractController.java b/app/src/main/java/com/limelight/binding/input/driver/AbstractController.java index df5c9a45..b2b4ea70 100644 --- a/app/src/main/java/com/limelight/binding/input/driver/AbstractController.java +++ b/app/src/main/java/com/limelight/binding/input/driver/AbstractController.java @@ -24,6 +24,17 @@ public abstract class AbstractController { private final int vendorId; private final int productId; + /** {@link #getPlayerNumber()} before the host has been told about this pad. */ + public static final int NO_PLAYER_NUMBER = -1; + + // The host's player number for this pad, 0-based. Assigned by ControllerHandler on the pad's + // first input report - not before, because the arrival it sends with it is lost if the + // stream is not up yet - and cleared when the pad goes away. Written on the driver thread, + // read on the UI thread by the game menu, hence volatile. Lives here rather than only on the + // handler's context so the menu, which holds the controller and nothing else, need not reach + // into the handler's maps from a thread that does not own them. + private volatile int playerNumber = NO_PLAYER_NUMBER; + private UsbDriverListener listener; // Current controller state, written by the subclass's parser and read by reportInput(). @@ -46,6 +57,20 @@ public int getControllerId() { return deviceId; } + /** + * @return the host's player number for this pad, 0-based, or {@link #NO_PLAYER_NUMBER} until + * its first input report has been sent. It is what the game on the host calls this + * pad, so it is what the game menu calls it too. + */ + public int getPlayerNumber() { + return playerNumber; + } + + /** For {@code ControllerHandler}, which owns the numbering. */ + public void setPlayerNumber(int playerNumber) { + this.playerNumber = playerNumber; + } + /** @return the USB vendor ID of the physical device */ public int getVendorId() { return vendorId; diff --git a/app/src/main/java/com/limelight/binding/input/driver/GipController.java b/app/src/main/java/com/limelight/binding/input/driver/GipController.java index 74c8da49..2a92079f 100644 --- a/app/src/main/java/com/limelight/binding/input/driver/GipController.java +++ b/app/src/main/java/com/limelight/binding/input/driver/GipController.java @@ -23,10 +23,24 @@ public class GipController extends AbstractController{ // Native controller instance owned by the driver, valid until the driver removes it private final long handle; - /** @param handle native controller instance supplied by the dongle's driver */ - public GipController(int deviceId, UsbDriverListener listener, int vendorId, int productId, long handle) { + // The user's guide LED choice, and the last command sent for it. Seeded with what + // startDevice() sends so a first battery report that maps to the same thing is not sent + // twice. Touched only on the driver's read thread - updateBattery() is the sole writer - so + // it needs no synchronisation. + private final GuideButtonLed guideButtonLed; + private GuideButtonLed.State ledSent; + + /** + * @param handle native controller instance supplied by the dongle's driver + * @param guideButtonLed the guide LED choice; its starting intensity has already gone to the + * native side with the transport, this is for what follows + */ + public GipController(int deviceId, UsbDriverListener listener, int vendorId, int productId, + long handle, GuideButtonLed guideButtonLed) { super(deviceId, listener, vendorId, productId); this.handle = handle; + this.guideButtonLed = guideButtonLed; + this.ledSent = guideButtonLed.startState(); // An Xbox pad however it is attached, so it declares what every other Xbox pad does. It // extends AbstractController rather than AbstractXboxController because the transport below @@ -89,9 +103,9 @@ public void updateInput(int buttons,short triggerLeft, short triggerRight, } /** - * Called from the native driver when the controller's battery state changes, with the raw GIP - * values. The mapping lives here rather than in the driver because the constants it maps onto - * are Moonlight's. + * Called from the native driver, on its read thread, when the controller's battery state + * changes, with the raw GIP values. The mapping lives here rather than in the driver because + * the constants it maps onto are Moonlight's. * *

The status byte packs four fields (MS-GIPUSB Table 30). Type says what kind of battery is * fitted, not whether it is charging — that is a separate field, which xow's struct @@ -99,11 +113,16 @@ public void updateInput(int buttons,short triggerLeft, short triggerRight, * battery at all: the pad is running off the cable. xow's enum calls that value * {@code BATT_TYPE_CHARGING}, which is a misnomer the spec settles. * + *

Also where the guide LED follows the battery, when the setting says so — see + * {@link #followBatteryLed}. + * * @param type GIP battery type: 0 absent or bus powered, 1 standard, 2 rechargeable * @param level GIP battery level, 0 to 3, meaningless when {@code type} is 0 * @param charge GIP charge state: 0 not charging, 1 charging, 2 charge error */ public void updateBattery(byte type, byte level, byte charge) { + followBatteryLed(type, level); + // No battery fitted: the pad is running off the cable, and its level means nothing if (type == 0) { reportBattery(MoonBridge.LI_BATTERY_STATE_NOT_CHARGING, @@ -135,6 +154,33 @@ public void updateBattery(byte type, byte level, byte charge) { reportBattery(state, percentage); } + /** + * Steps the guide LED with the battery, when the user's choice is to follow it. + * + *

Deduplicated against the last command sent because the driver reports on any change in + * the status byte - charge state and power level included - and only the level and the + * battery type move the LED. A send the pad refused is not recorded, so the next report + * retries it. + * + *

Runs inside the battery callback, on the driver's read thread: the same thread + * {@code startDevice()} sends the first LED command from, so this adds no new cross-thread + * sender to the native side. + */ + private void followBatteryLed(byte type, byte level) { + if (!guideButtonLed.followsBattery()) { + return; + } + + var next = GuideButtonLed.forBattery(type, level); + if (next.equals(ledSent)) { + return; + } + + if (setLedNative(handle, next.pattern(), next.intensity())) { + ledSent = next; + } + } + /** * @return true if this pad told us, in its metadata, that it can render 48 kHz stereo. A pad * that did not has no audio endpoint reachable this way, and enabling would move the @@ -239,5 +285,14 @@ public void forgetAudioDevice() { native void queueAudioNative(long handle, short[] samples, int count); native void sendRumble(long handle, short lowFreqMotor, short highFreqMotor); native void sendrumbleTriggers(long handle, short leftTrigger, short rightTrigger); + /** + * Sends a guide LED command, or stores it for {@code startDevice()} if the pad is not started + * yet. + * + * @param pattern one of {@code GuideButtonLed.PATTERN_*}; anything else is refused + * @param brightness 0 to {@code GuideButtonLed.INTENSITY_MAX}; clamped natively + * @return whether the pad took it, or it was stored to be sent at start + */ + native boolean setLedNative(long handle, int pattern, int brightness); } diff --git a/app/src/main/java/com/limelight/binding/input/driver/GuideButtonLed.java b/app/src/main/java/com/limelight/binding/input/driver/GuideButtonLed.java new file mode 100644 index 00000000..92b27701 --- /dev/null +++ b/app/src/main/java/com/limelight/binding/input/driver/GuideButtonLed.java @@ -0,0 +1,96 @@ +package com.limelight.binding.input.driver; + +/** + * The user's guide button LED choice for the Xbox pads this app drives, and the ladder that turns + * a battery level into an LED command when the choice is to follow the battery. + * + *

Every LED number on the Java side lives here. The values are the protocol's own, not + * percentages of anything we chose: MS-GIPUSB 3.1.5.5.7 Table 41 defines the LED command's + * intensity byte as 0 to 47 ({@code 0x2F}), and Table 42 its pattern byte. Keeping the whole + * translation on this side of the JNI boundary keeps it testable; the native side only applies + * what it is handed, and rejects a pattern it does not know. + * + *

Split out of {@link GipController}, which loads a native library in its static initialiser + * and so cannot be touched from a JVM test, following {@link StickCalibration}. Android-free. + * + *

Latency: none. Read once per pad at start and once per battery report, on the driver's own + * thread; never on the frame or input path. + * + * @param intensity what the pad is told at start, Table 41 units + * @param followsBattery whether battery reports move it after that + */ +public record GuideButtonLed(int intensity, boolean followsBattery) { + + // Table 41 intensities. The presets, and the ceiling. xow believed the maximum was 0x20, so + // whether a pad honours anything above that is an open hardware question - see + // HARDWARE_TESTING.md 22. + public static final int INTENSITY_OFF = 0x00; + public static final int INTENSITY_DIM = 0x0A; + public static final int INTENSITY_NORMAL = 0x14; + public static final int INTENSITY_BRIGHT = 0x2F; + public static final int INTENSITY_MAX = 0x2F; + + // Table 42 patterns. These mirror enum LedMode in app/src/main/jni/xow_driver/controller/gip.h + // and must stay equal to it: the native side accepts exactly this set and refuses anything + // else, which is what catches the two drifting apart. + public static final int PATTERN_OFF = 0x00; + public static final int PATTERN_ON = 0x01; + public static final int PATTERN_BLINK_FAST = 0x02; + public static final int PATTERN_BLINK_MED = 0x03; + public static final int PATTERN_BLINK_SLOW = 0x04; + public static final int PATTERN_FADE_SLOW = 0x08; + public static final int PATTERN_FADE_FAST = 0x09; + + /** What the driver sent before the setting existed, and what anything unrecognised gets. */ + public static final GuideButtonLed NORMAL = new GuideButtonLed(INTENSITY_NORMAL, false); + + /** + * One LED command's worth: what the pad is told to show. + * + * @param pattern one of {@code PATTERN_*} + * @param intensity 0 to {@link #INTENSITY_MAX} + */ + public record State(int pattern, int intensity) { + } + + /** + * @return what the driver sends when the pad starts. Pattern and intensity are separate + * fields (Table 42), so an intensity of zero on a lit pattern is not what the spec + * means by off: zero selects the off pattern rather than a maximally dimmed on one. + */ + public State startState() { + return new State(intensity == 0 ? PATTERN_OFF : PATTERN_ON, intensity); + } + + /** + * The ladder: the LED for a battery report, when the setting is to follow it. + * + *

The steps are the presets, so each one is a brightness the user has already seen on the + * other settings: full is Bright, medium is Normal, low is Dim. Critical flashes rather than + * dimming further, because a dim LED across a room is indistinguishable from the Dim preset + * and a warning has to look like one; it flashes at Normal so the flash is visible. A pad with + * no battery fitted is running off its cable, and gets full brightness for the same reason a + * full battery does. + * + *

Only critical uses a pattern other than On. Table 42 marks every other pattern "not + * implemented by host", which describes Microsoft's host rather than the pad, so whether a + * pad honours the slow blink is a hardware question - HARDWARE_TESTING.md 31. + * + * @param type GIP battery type: 0 absent or bus powered, 1 standard, 2 rechargeable + * @param level GIP battery level, 0 critical to 3 full; meaningless when {@code type} is 0 + * @return the command to send; a level the spec does not define gets what the pad was + * started with, so an unexpected value never changes anything + */ + public static State forBattery(byte type, byte level) { + if (type == 0) { + return new State(PATTERN_ON, INTENSITY_BRIGHT); + } + return switch (level) { + case 3 -> new State(PATTERN_ON, INTENSITY_BRIGHT); + case 2 -> new State(PATTERN_ON, INTENSITY_NORMAL); + case 1 -> new State(PATTERN_ON, INTENSITY_DIM); + case 0 -> new State(PATTERN_BLINK_SLOW, INTENSITY_NORMAL); + default -> NORMAL.startState(); + }; + } +} diff --git a/app/src/main/java/com/limelight/binding/input/driver/UsbDriverListener.java b/app/src/main/java/com/limelight/binding/input/driver/UsbDriverListener.java index 5d76f45b..961812b9 100644 --- a/app/src/main/java/com/limelight/binding/input/driver/UsbDriverListener.java +++ b/app/src/main/java/com/limelight/binding/input/driver/UsbDriverListener.java @@ -33,7 +33,9 @@ void reportControllerState(int controllerId, int buttonFlags, /** * Reports a change in the controller's battery. Sent only when the level actually moves, not - * on a timer, so implementations should not expect a steady cadence. + * on a timer, so implementations should not expect a steady cadence. It may also arrive before + * the controller's first input report, so an implementation that needs a player number for it + * has to hold on to it. * * @param batteryState one of {@code MoonBridge.LI_BATTERY_STATE_*} * @param batteryPercentage 0 to 100, or {@code MoonBridge.LI_BATTERY_PERCENTAGE_UNKNOWN} diff --git a/app/src/main/java/com/limelight/binding/input/driver/XboxWiredGipController.java b/app/src/main/java/com/limelight/binding/input/driver/XboxWiredGipController.java index 0b5ae433..f0898fb2 100644 --- a/app/src/main/java/com/limelight/binding/input/driver/XboxWiredGipController.java +++ b/app/src/main/java/com/limelight/binding/input/driver/XboxWiredGipController.java @@ -63,8 +63,10 @@ private static UsbInterface findGipInterface(UsbDevice device) { private XboxWiredGipController(UsbDevice device, UsbDeviceConnection connection, UsbInterface gipInterface, int deviceId, - UsbDriverListener listener, long wiredHandle, long gipHandle) { - super(deviceId, listener, device.getVendorId(), device.getProductId(), gipHandle); + UsbDriverListener listener, long wiredHandle, long gipHandle, + GuideButtonLed guideButtonLed) { + super(deviceId, listener, device.getVendorId(), device.getProductId(), gipHandle, + guideButtonLed); this.wiredHandle = wiredHandle; this.connection = connection; @@ -74,16 +76,17 @@ private XboxWiredGipController(UsbDevice device, UsbDeviceConnection connection, /** * Claims the pad and brings the GIP stack up on it. * - * @param ledBrightness guide button LED intensity, as the raw protocol value. Passed at - * transport creation rather than set afterwards because the native side - * sends the LED command from {@code startDevice()}, which can run before - * this object exists — see {@link XboxWirelessDongle}. + * @param guideButtonLed guide button LED choice. Its starting intensity is passed at transport + * creation rather than set afterwards because the native side sends the + * LED command from {@code startDevice()}, which can run before this + * object exists — see {@link XboxWirelessDongle}. The battery moves it + * from here on, if the choice says so. * @return the controller, or null if the device could not be claimed — in which case nothing * has been left running and the caller still owns {@code connection} */ public static XboxWiredGipController create(UsbDevice device, UsbDeviceConnection connection, int deviceId, UsbDriverListener listener, - int ledBrightness) { + GuideButtonLed guideButtonLed) { /* * Claimed here rather than natively, and forced. Android's own driver holds this interface @@ -103,7 +106,7 @@ public static XboxWiredGipController create(UsbDevice device, UsbDeviceConnectio return null; } - long wired = createWiredDriver(connection.getFileDescriptor(), ledBrightness); + long wired = createWiredDriver(connection.getFileDescriptor(), guideButtonLed.intensity()); if (wired == 0) { LimeLog.warning("Wired GIP: could not create the native transport"); connection.releaseInterface(gipInterface); @@ -128,7 +131,7 @@ public static XboxWiredGipController create(UsbDevice device, UsbDeviceConnectio } return new XboxWiredGipController(device, connection, gipInterface, deviceId, listener, - wired, gip); + wired, gip, guideButtonLed); } /** diff --git a/app/src/main/java/com/limelight/binding/input/driver/XboxWirelessDongle.java b/app/src/main/java/com/limelight/binding/input/driver/XboxWirelessDongle.java index a7e42059..a7fa8395 100644 --- a/app/src/main/java/com/limelight/binding/input/driver/XboxWirelessDongle.java +++ b/app/src/main/java/com/limelight/binding/input/driver/XboxWirelessDongle.java @@ -33,14 +33,16 @@ public class XboxWirelessDongle { private long driverHandle; /* - * Guide button LED intensity handed to every pad this adapter brings up. + * Guide button LED choice handed to every pad this adapter brings up. * - * Held here, and passed at driver creation rather than set on each pad afterwards, because the - * native side sends the LED command from startDevice() as soon as a pad's metadata arrives - - * which can be before the Java GipController for that pad exists. A setter called after - * construction would race that first packet and lose on a fast pairing. + * The starting intensity is passed at driver creation rather than set on each pad afterwards, + * because the native side sends the LED command from startDevice() as soon as a pad's metadata + * arrives - which can be before the Java GipController for that pad exists, so a setter called + * after construction would race that first packet and lose on a fast pairing. Whether the + * battery moves it after that is the pad's own business: each GipController gets the whole + * choice and drives its LED from its battery reports. */ - private final int ledBrightness; + private final GuideButtonLed guideButtonLed; // Live controllers by the native driver's slot index private Map controllers = new HashMap<>(); @@ -57,17 +59,17 @@ public class XboxWirelessDongle { } /** - * @param connection an already-open connection, whose file descriptor is handed to the native driver - * @param ledBrightness guide button LED intensity for this adapter's pads, as the raw protocol - * value — see {@code PreferenceConfiguration.getGuideButtonLedValue} + * @param connection an already-open connection, whose file descriptor is handed to the native driver + * @param guideButtonLed guide button LED choice for this adapter's pads — see + * {@code PreferenceConfiguration.getGuideButtonLedValue} */ public XboxWirelessDongle(UsbDevice device, UsbDeviceConnection connection, - UsbDriverListener listener, int ledBrightness) { + UsbDriverListener listener, GuideButtonLed guideButtonLed) { this.device = device; this.connection = connection; this.listener = listener; this.driverHandle = -1; - this.ledBrightness = ledBrightness; + this.guideButtonLed = guideButtonLed; } /** @@ -84,7 +86,7 @@ public synchronized boolean start() { if(this.driverHandle != -1) { return false; //we already started; } - this.driverHandle = createDriver(connection.getFileDescriptor(), ledBrightness); + this.driverHandle = createDriver(connection.getFileDescriptor(), guideButtonLed.intensity()); boolean ok = startDriver(this.driverHandle, ""); if(!ok) { LimeLog.info("xbox wireless dongle driver failed to start"); @@ -168,7 +170,7 @@ public static boolean canClaimDevice(UsbDevice device) { public void addNewController(int id, long handle, short vid, short pid, long address){ // Namespaced by vendor ID so pad numbers can't collide with other drivers' device IDs var controller = new GipController(numberFor(address) + 0x045e0000, - listener, vid, pid, handle); + listener, vid, pid, handle, guideButtonLed); controllers.put(id, controller); this.listener.deviceAdded(controller); } diff --git a/app/src/main/java/com/limelight/preferences/PreferenceConfiguration.java b/app/src/main/java/com/limelight/preferences/PreferenceConfiguration.java index 4036a0f6..20a17a58 100644 --- a/app/src/main/java/com/limelight/preferences/PreferenceConfiguration.java +++ b/app/src/main/java/com/limelight/preferences/PreferenceConfiguration.java @@ -6,6 +6,7 @@ import java.util.Locale; +import com.limelight.binding.input.driver.GuideButtonLed; import com.limelight.nvstream.StreamConfiguration; import com.limelight.nvstream.jni.MoonBridge; @@ -162,10 +163,13 @@ public enum AnalogStickForScrolling { /** Whether to drive cabled Xbox pads ourselves so their headphone jack can take audio. */ public boolean wiredPadAudio; /** - * Guide button LED intensity for Xbox pads this app drives, as the raw protocol value the - * pad expects. Only reaches a pad the GIP driver owns — see {@code UsbDriverService}. + * Guide button LED choice for Xbox pads this app drives: the intensity to start at, in the + * pad's own units, and whether the battery moves it after that. Only reaches a pad the GIP + * driver owns — see {@code UsbDriverService}. The type lives in the driver package because its + * numbers are the GIP protocol's; this package importing it is the only dependency between + * the two, and the record has no static initialiser, so nothing loads that should not. */ - public int guideButtonLed; + public GuideButtonLed guideButtonLed; public boolean mouseEmulation; public AnalogStickForScrolling analogStickForScrolling; public boolean mouseNavButtons; @@ -366,34 +370,37 @@ static int getEncryptionFlagsValue(String value) { } /** - * Maps the stored preference value to the guide button LED intensity the pad expects. + * Maps the stored preference value to the guide button LED choice the driver applies. * - *

The returned number is the protocol's own field, not a percentage of anything we chose: + *

The intensity is the protocol's own field, not a percentage of anything we chose: * MS-GIPUSB 3.1.5.5.7 Table 41 defines the LED command's third payload byte as an intensity * of 0 to 47 ({@code 0x2F}). Mapping presets to that value here, rather than in the driver, * keeps the whole translation on the testable side of the JNI boundary — the native side - * only applies what it is handed. + * only applies what it is handed, pattern included, and {@link GuideButtonLed} holds the + * numbers. + * + *

"battery" starts at Normal, because the pad has not said what it has yet, and lets each + * battery report move it from there. * *

Split out and static so it can be unit tested, following * {@link #getEncryptionFlagsValue(String)}. * * @param value the stored string, or null if the preference has never been written - * @return the intensity to send, defaulting to the value the driver used before this setting + * @return the choice to apply, defaulting to the value the driver used before this setting * existed */ - static int getGuideButtonLedValue(String value) { + static GuideButtonLed getGuideButtonLedValue(String value) { if (value == null) { value = DEFAULT_GUIDE_BUTTON_LED; } return switch (value) { - case "off" -> 0x00; - case "dim" -> 0x0A; - // The spec's ceiling. xow believed the maximum was 0x20, so whether a pad honours - // anything above that is an open hardware question - see HARDWARE_TESTING.md 22. - case "bright" -> 0x2F; + case "off" -> new GuideButtonLed(GuideButtonLed.INTENSITY_OFF, false); + case "dim" -> new GuideButtonLed(GuideButtonLed.INTENSITY_DIM, false); + case "bright" -> new GuideButtonLed(GuideButtonLed.INTENSITY_BRIGHT, false); + case "battery" -> new GuideButtonLed(GuideButtonLed.INTENSITY_NORMAL, true); // "normal", plus anything unrecognised - default -> 0x14; + default -> GuideButtonLed.NORMAL; }; } diff --git a/app/src/main/java/com/limelight/utils/DialogChain.java b/app/src/main/java/com/limelight/utils/DialogChain.java new file mode 100644 index 00000000..33bd4998 --- /dev/null +++ b/app/src/main/java/com/limelight/utils/DialogChain.java @@ -0,0 +1,35 @@ +package com.limelight.utils; + +/** + * Which dialog in a chain of replacements is the live one. + * + *

The in-game menu is a chain of {@link MenuDialog}s: each submenu is its own dialog, opened + * from a row of the one above, and Back reopens the parent as yet another new dialog. Something + * shown for the whole menu — the battery label — has to go away when the chain ends, not + * when any one dialog does, and the framework's ordering makes that harder than it sounds: + * {@code AlertController} runs a row's click listener before it dismisses the dialog, and + * {@code onDismiss} is delivered as a posted message. So the child opens first and only then does + * the parent's {@code onDismiss} arrive. Hiding on every dismiss hid the label the moment any + * submenu opened. + * + *

Each dialog takes a token when it opens; its {@code onDismiss} asks whether it is still the + * latest. Only the last dialog in the chain is, so only it ends the chain. + * + *

Main thread only — the menu opens and dismisses there and nowhere else. + */ +public final class DialogChain { + private int generation; + + /** Records a dialog opening. @return the token its {@code onDismiss} must present. */ + public int opened() { + return ++generation; + } + + /** + * @return true if no dialog has opened since {@code token} was issued, which is to say this + * dismissal ends the chain + */ + public boolean closes(int token) { + return token == generation; + } +} diff --git a/app/src/main/java/com/limelight/utils/MenuDialog.java b/app/src/main/java/com/limelight/utils/MenuDialog.java index 6f2f8d31..64c6cc7f 100644 --- a/app/src/main/java/com/limelight/utils/MenuDialog.java +++ b/app/src/main/java/com/limelight/utils/MenuDialog.java @@ -57,7 +57,9 @@ public static void show(Activity activity, String title, List

{@code MoonBridge.LI_BATTERY_*} are compile-time constants, so referencing them here inlines + * them and never loads {@code MoonBridge}, whose static initialiser would call + * {@code System.loadLibrary}. Nothing else in that class may be touched from a test. + */ +class ControllerBatteriesTest { + + private static final byte UNKNOWN = MoonBridge.LI_BATTERY_PERCENTAGE_UNKNOWN; + + @Nested + @DisplayName("classify()") + class Classification { + + /** + * The whole table. The unknown-percentage rows are the ones worth reading: a GIP pad with + * no battery pack reports {@code NOT_CHARGING} and no percentage, and the only fact in + * that is the cable, which is still worth a word; anything else without a percentage + * says nothing. Android can report {@code UNKNOWN} with a real capacity, which is a + * level, not nothing. + */ + @ParameterizedTest(name = "state {0} percentage {1} -> {2}") + @CsvSource({ + "2, 75, LEVEL", // discharging + "5, 100, LEVEL", // full + "0, 40, LEVEL", // unknown state, known level + "4, 60, LEVEL", // not charging, known level + "3, 50, CHARGING", + "4, -1, WIRED", // not charging, no level: a pad on its cable + "3, -1, WIRED", // charging, no level + "0, -1, HIDDEN", + "2, -1, HIDDEN", + "1, 50, HIDDEN", // not present + }) + @DisplayName("each reading classifies as the table says") + void classifiesAsTheTableSays(byte state, byte percentage, Display expected) { + assertEquals(expected, ControllerBatteries.classify(state, percentage)); + } + } + + @Nested + @DisplayName("snapshot()") + class Snapshot { + + @Test + @DisplayName("an empty store yields an empty list") + void emptyStoreYieldsEmptyList() { + assertTrue(new ControllerBatteries().snapshot().isEmpty()); + } + + /** + * Several pads at once is the case the label exists for. They come back in player order + * whatever order they reported in, each with its own display kind. + */ + @Test + @DisplayName("mixed controllers come back in player order") + void mixedControllersComeBackInPlayerOrder() { + ControllerBatteries batteries = new ControllerBatteries(); + batteries.update(3, MoonBridge.LI_BATTERY_STATE_NOT_CHARGING, UNKNOWN); + batteries.update(0, MoonBridge.LI_BATTERY_STATE_DISCHARGING, (byte) 75); + batteries.update(2, MoonBridge.LI_BATTERY_STATE_CHARGING, (byte) 50); + + assertEquals(List.of( + new Reading(0, Display.LEVEL, 75), + new Reading(2, Display.CHARGING, 50), + new Reading(3, Display.WIRED, UNKNOWN & 0xFF)), + batteries.snapshot()); + } + + /** A reading with nothing to say is not a row; the label would only print "unknown". */ + @Test + @DisplayName("readings with nothing to show are excluded") + void unknownReadingsAreExcluded() { + ControllerBatteries batteries = new ControllerBatteries(); + batteries.update(0, MoonBridge.LI_BATTERY_STATE_DISCHARGING, UNKNOWN); + batteries.update(1, MoonBridge.LI_BATTERY_STATE_NOT_PRESENT, (byte) 50); + + assertTrue(batteries.snapshot().isEmpty()); + } + + /** A pad that unplugged must not linger on the label under its old number. */ + @Test + @DisplayName("a cleared controller disappears") + void removedControllerDisappears() { + ControllerBatteries batteries = new ControllerBatteries(); + batteries.update(1, MoonBridge.LI_BATTERY_STATE_DISCHARGING, (byte) 75); + batteries.clear(1); + + assertTrue(batteries.snapshot().isEmpty()); + } + + /** The number a pad vacated goes to the next pad; the slot must take the new reading. */ + @Test + @DisplayName("a re-added controller reuses its slot") + void reAddedControllerReusesItsSlot() { + ControllerBatteries batteries = new ControllerBatteries(); + batteries.update(1, MoonBridge.LI_BATTERY_STATE_DISCHARGING, (byte) 75); + batteries.clear(1); + batteries.update(1, MoonBridge.LI_BATTERY_STATE_CHARGING, (byte) 20); + + assertEquals(List.of(new Reading(1, Display.CHARGING, 20)), batteries.snapshot()); + } + + @Test + @DisplayName("the latest update wins") + void latestUpdateWins() { + ControllerBatteries batteries = new ControllerBatteries(); + batteries.update(0, MoonBridge.LI_BATTERY_STATE_DISCHARGING, (byte) 75); + batteries.update(0, MoonBridge.LI_BATTERY_STATE_DISCHARGING, (byte) 74); + + assertEquals(List.of(new Reading(0, Display.LEVEL, 74)), batteries.snapshot()); + } + + /** + * The percentage travels as a Java byte, which is signed. 100 fits, but the arithmetic + * that reads it back must not sign-extend anything, or a value over 127 - which the + * unknown marker is - would come back negative. + */ + @Test + @DisplayName("the percentage is read unsigned") + void percentageIsUnsigned() { + ControllerBatteries batteries = new ControllerBatteries(); + batteries.update(0, MoonBridge.LI_BATTERY_STATE_FULL, (byte) 100); + batteries.update(1, MoonBridge.LI_BATTERY_STATE_CHARGING, UNKNOWN); + + List readings = batteries.snapshot(); + assertEquals(100, readings.get(0).percentage()); + assertEquals(0xFF, readings.get(1).percentage()); + } + + /** + * Fed from driver threads, where an exception takes the pad's read loop down. A number + * outside the slots is dropped, not thrown on, and leaves nothing behind. + */ + @Test + @DisplayName("out-of-range controller numbers are ignored") + void outOfRangeNumbersAreIgnored() { + ControllerBatteries batteries = new ControllerBatteries(); + batteries.update(-1, MoonBridge.LI_BATTERY_STATE_DISCHARGING, (byte) 75); + batteries.update(ControllerBatteries.MAX_CONTROLLERS, + MoonBridge.LI_BATTERY_STATE_DISCHARGING, (byte) 75); + batteries.clear(-1); + batteries.clear(ControllerBatteries.MAX_CONTROLLERS); + + assertTrue(batteries.snapshot().isEmpty()); + } + } +} diff --git a/app/src/test/java/com/limelight/binding/input/driver/GuideButtonLedTest.java b/app/src/test/java/com/limelight/binding/input/driver/GuideButtonLedTest.java new file mode 100644 index 00000000..dd028a88 --- /dev/null +++ b/app/src/test/java/com/limelight/binding/input/driver/GuideButtonLedTest.java @@ -0,0 +1,153 @@ +package com.limelight.binding.input.driver; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Nested; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.CsvSource; + +/** + * Tests for the guide LED numbers and the battery ladder. + * + *

Everything here goes to the pad byte for byte, so the values are pinned rather than + * described: a step that silently changed would otherwise only be visible on hardware, and the + * hardware is the one place these are hard to check. + */ +class GuideButtonLedTest { + + @Nested + @DisplayName("forBattery()") + class Ladder { + + /** + * The ladder in full. Type 1 and 2 are the two kinds of battery the spec knows; type 0 is + * no battery at all, whose level the spec says to ignore. Level 7 does not exist and must + * change nothing, which means the start state rather than the brightest or dimmest step. + */ + @ParameterizedTest(name = "type {0} level {1} -> pattern {2} intensity {3}") + @CsvSource({ + "1, 3, 0x01, 0x2F", + "1, 2, 0x01, 0x14", + "1, 1, 0x01, 0x0A", + "1, 0, 0x04, 0x14", + "2, 0, 0x04, 0x14", + "0, 0, 0x01, 0x2F", + "0, 3, 0x01, 0x2F", + "1, 7, 0x01, 0x14", + }) + @DisplayName("each battery report maps to its LED command") + void ladderIsPinned(byte type, byte level, int pattern, int intensity) { + assertEquals(new GuideButtonLed.State(pattern, intensity), + GuideButtonLed.forBattery(type, level)); + } + + /** + * The flash is the warning. Any other level flashing would make the warning meaningless, + * and a flashing full pad would send the user hunting for a charger. + */ + @Test + @DisplayName("only the critical level uses a pattern other than on") + void onlyCriticalBlinks() { + for (byte level = 1; level <= 3; level++) { + assertEquals(GuideButtonLed.PATTERN_ON, + GuideButtonLed.forBattery((byte) 1, level).pattern(), + "level " + level + " should be steady"); + } + assertEquals(GuideButtonLed.PATTERN_ON, + GuideButtonLed.forBattery((byte) 0, (byte) 0).pattern(), + "a pad on its cable should be steady"); + assertNotEquals(GuideButtonLed.PATTERN_ON, + GuideButtonLed.forBattery((byte) 1, (byte) 0).pattern()); + } + + /** + * A pad about to die must still show something: an LED that went dark at critical would + * read as the pad having already gone. + */ + @Test + @DisplayName("critical is not off") + void criticalIsNotOff() { + GuideButtonLed.State critical = GuideButtonLed.forBattery((byte) 1, (byte) 0); + + assertNotEquals(GuideButtonLed.PATTERN_OFF, critical.pattern()); + assertTrue(critical.intensity() > 0); + } + + /** The steady steps dim as the battery drains; a brighter step for less charge is a bug. */ + @Test + @DisplayName("intensity never rises as the level drops") + void intensityNeverRisesAsTheLevelDrops() { + int previous = Integer.MAX_VALUE; + for (byte level = 3; level >= 1; level--) { + int intensity = GuideButtonLed.forBattery((byte) 1, level).intensity(); + assertTrue(intensity <= previous, + "level " + level + " is brighter than the level above it"); + previous = intensity; + } + } + + /** Nothing may exceed what the protocol's intensity field can carry, whatever the input. */ + @Test + @DisplayName("every state is within Table 41's range") + void everyStateIsWithinTable41() { + for (byte type = 0; type <= 3; type++) { + for (byte level = 0; level <= 3; level++) { + int intensity = GuideButtonLed.forBattery(type, level).intensity(); + assertTrue(intensity >= 0 && intensity <= GuideButtonLed.INTENSITY_MAX, + "type " + type + " level " + level + " gives " + intensity); + } + } + } + } + + @Nested + @DisplayName("startState()") + class StartState { + + /** + * Pattern and intensity are separate fields (Table 42), so off has to be the off pattern: + * an intensity of zero on the on pattern is a maximally dimmed on, and what a pad does + * with that is not something the spec promises. + */ + @Test + @DisplayName("zero intensity selects the off pattern") + void zeroIntensitySelectsTheOffPattern() { + assertEquals(new GuideButtonLed.State(GuideButtonLed.PATTERN_OFF, 0), + new GuideButtonLed(0, false).startState()); + } + + @Test + @DisplayName("any other intensity selects the on pattern") + void nonZeroSelectsOn() { + assertEquals(new GuideButtonLed.State(GuideButtonLed.PATTERN_ON, 0x14), + new GuideButtonLed(0x14, true).startState()); + } + } + + @Nested + @DisplayName("constants") + class Constants { + + /** + * These cross the JNI boundary as plain ints and are matched against {@code enum LedMode} + * in {@code app/src/main/jni/xow_driver/controller/gip.h} by value. The native side + * refuses a value outside that enum, so a drift would show as a refused command on + * hardware; this pins the Java half so it shows here first. + */ + @Test + @DisplayName("patterns match enum LedMode in gip.h") + void patternsMatchGipHeader() { + assertEquals(0x00, GuideButtonLed.PATTERN_OFF); + assertEquals(0x01, GuideButtonLed.PATTERN_ON); + assertEquals(0x02, GuideButtonLed.PATTERN_BLINK_FAST); + assertEquals(0x03, GuideButtonLed.PATTERN_BLINK_MED); + assertEquals(0x04, GuideButtonLed.PATTERN_BLINK_SLOW); + assertEquals(0x08, GuideButtonLed.PATTERN_FADE_SLOW); + assertEquals(0x09, GuideButtonLed.PATTERN_FADE_FAST); + } + } +} diff --git a/app/src/test/java/com/limelight/preferences/PreferenceConfigurationTest.java b/app/src/test/java/com/limelight/preferences/PreferenceConfigurationTest.java index 7166c92f..bdfb6cd4 100644 --- a/app/src/test/java/com/limelight/preferences/PreferenceConfigurationTest.java +++ b/app/src/test/java/com/limelight/preferences/PreferenceConfigurationTest.java @@ -10,6 +10,7 @@ import org.junit.jupiter.params.provider.CsvSource; import org.junit.jupiter.params.provider.ValueSource; +import com.limelight.binding.input.driver.GuideButtonLed; import com.limelight.nvstream.StreamConfiguration; /** @@ -75,50 +76,75 @@ void matchesPreferenceEntryValues() { @Nested @DisplayName("guide button LED brightness") - class GuideButtonLed { + class GuideButtonLedSetting { /** * The values are the protocol's, not ours: MS-GIPUSB 3.1.5.5.7 Table 41 defines the LED * command's intensity byte as 0 to 47. Pinning them here is the point of the test — a - * preset that silently changed value would otherwise only be visible on hardware. + * preset that silently changed value would otherwise only be visible on hardware. The + * battery setting starts at normal, since the pad has not said what it has yet; the + * ladder that moves it from there is {@code GuideButtonLedTest}'s to pin. */ - @ParameterizedTest(name = "\"{0}\" maps to {1}") + @ParameterizedTest(name = "\"{0}\" starts at {1}") @CsvSource({ - "off, 0x00", - "dim, 0x0A", - "normal, 0x14", - "bright, 0x2F", + "off, 0x00", + "dim, 0x0A", + "normal, 0x14", + "bright, 0x2F", + "battery, 0x14", }) - @DisplayName("each preset maps to its protocol intensity") + @DisplayName("each preset maps to its starting protocol intensity") void presetsMapToIntensity(String stored, int expected) { - assertEquals(expected, PreferenceConfiguration.getGuideButtonLedValue(stored)); + assertEquals(expected, + PreferenceConfiguration.getGuideButtonLedValue(stored).intensity()); + } + + /** + * Only the battery setting lets battery reports move the LED. A fixed preset that + * followed the battery would drift away from what the user chose; a battery setting that + * did not would be the normal preset under another name. + */ + @ParameterizedTest(name = "\"{0}\" follows the battery: {1}") + @CsvSource({ + "off, false", + "dim, false", + "normal, false", + "bright, false", + "battery, true", + }) + @DisplayName("only the battery setting follows the battery") + void onlyBatteryFollowsTheBattery(String stored, boolean expected) { + assertEquals(expected, + PreferenceConfiguration.getGuideButtonLedValue(stored).followsBattery()); } /** * A preference the user has never touched reads back as null, and a value written by a * future build might be anything. Both have to land on the intensity the driver used - * before this was configurable, rather than leaving the LED off or at full brightness. + * before this was configurable, rather than leaving the LED off or at full brightness — + * and must not follow the battery, which nobody asked for. */ @Test - @DisplayName("null falls back to the normal intensity") + @DisplayName("null falls back to the normal preset") void nullFallsBackToNormal() { - assertEquals(0x14, PreferenceConfiguration.getGuideButtonLedValue(null)); + assertEquals(GuideButtonLed.NORMAL, PreferenceConfiguration.getGuideButtonLedValue(null)); } @Test - @DisplayName("an unrecognised value falls back to the normal intensity") + @DisplayName("an unrecognised value falls back to the normal preset") void unrecognisedFallsBackToNormal() { - assertEquals(0x14, PreferenceConfiguration.getGuideButtonLedValue("chartreuse")); + assertEquals(GuideButtonLed.NORMAL, + PreferenceConfiguration.getGuideButtonLedValue("chartreuse")); } /** Nothing may exceed what the protocol's intensity field can carry. */ @ParameterizedTest - @CsvSource({"off", "dim", "normal", "bright"}) + @CsvSource({"off", "dim", "normal", "bright", "battery"}) @DisplayName("no preset exceeds the protocol maximum of 0x2F") void presetsStayWithinProtocolRange(String stored) { - int intensity = PreferenceConfiguration.getGuideButtonLedValue(stored); + int intensity = PreferenceConfiguration.getGuideButtonLedValue(stored).intensity(); - assertTrue(intensity >= 0x00 && intensity <= 0x2F, + assertTrue(intensity >= 0x00 && intensity <= GuideButtonLed.INTENSITY_MAX, "intensity " + intensity + " is outside the protocol's 0x00-0x2F range"); } } diff --git a/app/src/test/java/com/limelight/utils/DialogChainTest.java b/app/src/test/java/com/limelight/utils/DialogChainTest.java new file mode 100644 index 00000000..91a39e38 --- /dev/null +++ b/app/src/test/java/com/limelight/utils/DialogChainTest.java @@ -0,0 +1,72 @@ +package com.limelight.utils; + +import static org.junit.jupiter.api.Assertions.assertFalse; +import static org.junit.jupiter.api.Assertions.assertTrue; + +import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; + +/** + * Tests for the token that tells the last dialog in the in-game menu's chain from the rest. + * + *

Each scenario is a sequence the menu actually produces; the order of opens and dismissals + * follows the framework's, where a row's action opens the child before the parent's dismissal is + * delivered. + */ +class DialogChainTest { + + /** Cancel on the root, or Back at the root: nothing else opened, so this ends the chain. */ + @Test + @DisplayName("dismissing the only dialog ends the chain") + void rootDismissEndsTheChain() { + DialogChain chain = new DialogChain(); + int root = chain.opened(); + + assertTrue(chain.closes(root)); + } + + /** + * A row opens a submenu. The child has opened by the time the parent's dismissal arrives, so + * the parent must not end the chain - that is the case that hid the label the moment any + * submenu opened. + */ + @Test + @DisplayName("a parent's dismissal after its child opened does not end the chain") + void parentDismissAfterChildOpensDoesNotEndIt() { + DialogChain chain = new DialogChain(); + int root = chain.opened(); + int child = chain.opened(); + + assertFalse(chain.closes(root)); + assertTrue(chain.closes(child)); + } + + /** + * Back from a submenu reopens the parent as a new dialog, not the old one. The reopened + * parent is then the live dialog and the submenu's own dismissal must not end the chain. + */ + @Test + @DisplayName("reopening the parent on Back is a new generation") + void backReopensParentAsNewGeneration() { + DialogChain chain = new DialogChain(); + chain.opened(); // root + int child = chain.opened(); + int reopenedRoot = chain.opened(); // Back: root again, as a fresh dialog + + assertFalse(chain.closes(child)); + assertTrue(chain.closes(reopenedRoot)); + } + + /** A token from further back in the chain never closes it, however many opens later. */ + @Test + @DisplayName("a stale token never ends the chain") + void staleTokenNeverCloses() { + DialogChain chain = new DialogChain(); + int first = chain.opened(); + for (int i = 0; i < 5; i++) { + chain.opened(); + } + + assertFalse(chain.closes(first)); + } +}