Fix/android auto button slots - #108
Conversation
Button placement never reached anything a user could see. Two paths published buttons: the notification path assigned Media3 slots and sent them per-controller, while every other surface got a hardcoded, slot-less [JUMP_BACKWARD, JUMP_FORWARD, FAVORITE] list broadcast with the pre-1.7 setCustomLayout. Android Auto connects as a legacy controller and reads the platform PlaybackState, which Media3 derives from the session-wide layout — the slot-less one. Since Android 13 the shade renders the system media controls from that same PlaybackState and ignores the notification's actions, so `notificationButtons` governed neither the car nor a modern phone. Collapse to a single slot-annotated layout and publish it session-wide via setMediaButtonPreferences, plus the notification-controller overload — only that one calls updateLegacySessionPlaybackState, so without it a reordering that leaves the slot-reservation flags untouched sits invisible until an unrelated playback event republishes the state. Drop the secondary slots. Media3 defines SLOT_BACK_SECONDARY and SLOT_FORWARD_SECONDARY but DisplayConstraints defaults both to 0 buttons, and every system surface flattens the layout to back, forward and overflow, so they rendered nowhere. Each remaining slot names SLOT_OVERFLOW as a fallback, since the flattening keeps a button only if it won a primary slot or contains overflow. Commands now gate on capabilities alone: omitting a button from the layout removes it from every screen but leaves it working from a Bluetooth remote or headset. Placing a button in a primary slot is what makes Media3 strip ACTION_SKIP_TO_PREVIOUS/NEXT from the legacy state, so no command has to be revoked to free those positions. Also fixed along the way: - The heart blanked on every updateOptions() call. updateMediaSession took `favorited` with a null default that no caller passed, so it rebuilt the layout as un-favorited and stayed wrong until the next track change. - Jump buttons carried only a drawable, leaving the icon constant at ICON_UNDEFINED. They now use ICON_SKIP_BACK/FORWARD_5/10/15/30 chosen from the configured interval, which is what a controller rendering its own UI reads. - Layout change detection compared a generated data class whose `overflow` is an Array, so Kotlin's referential array equality reported "changed" on every options update and republished to the car. BREAKING CHANGE: `android.notificationButtons` is now `android.remoteButtonLayout`, and its `backSecondary` / `forwardSecondary` fields are gone — no Android surface ever rendered them. The type `NotificationButtonLayout` is now `RemoteButtonLayout` and `NotificationButton` is now `RemoteButton`; nothing in this path is notification-specific, which is what disguised the bug. Buttons that previously named a secondary slot should move to `overflow`.
|
Addresses #107 |
puckey
left a comment
There was a problem hiding this comment.
Note: Claude wrote this comment. I reviewed it before posting.
Thank you for this. The diagnosis and the mechanism are correct. I have one defect to report, three small items, and one confirmation.
1. The per-field rule is not possible
The doc comment on RemoteButtonLayout gives this rule:
Per field: omit it to derive from capabilities, set
nullto leave it empty, or name a button to place it there.
deriveButtonSlots does not apply this rule. When layout is not null, the function reads back, forward and overflow. It adds nothing more. An omitted field and a null field give the same result.
The bridge is the cause. I made a probe struct with the nitrogen version in this repository. A field a?: MediaButton | null becomes val a: MediaButton? in Kotlin. The two states become one. Nitro keeps the difference for arrays, structs and numbers, but not for a field whose type is an enum. RemoteButton is an enum. Thus the native code cannot know if the app omitted the field or set it to null.
The effect on users
website/guide/favorites.md gives this example:
android: { remoteButtonLayout: { overflow: ['favorite'] } }The table below it says that this example adds a heart to the notification, to Android Auto and to the system media controls. But this layout contains only the heart. The skip buttons and the jump buttons are not in the layout. They do not appear on any External surface. The Capabilities do not change this result.
The old code has the same behaviour. But the old layout controlled only the notification. This PR makes one layout control all External surfaces. Thus an incomplete layout now has a much larger effect.
The example in the setupPlayer doc comment has the same problem. It omits overflow, so the favorite heart does not appear.
The proposed correction
Make all three fields necessary. Then remove the per-field rule from the doc comment.
export type RemoteButtonLayout = {
/** Left of play/pause. `undefined` leaves it empty. */
back: RemoteButton | undefined
/** Right of play/pause. `undefined` leaves it empty. */
forward: RemoteButton | undefined
/** Everything else, most important first. `[]` for none. */
overflow: RemoteButton[]
}This correction has four results:
- Each field has two states, not three. The bridge can carry two states.
- The wire format does not change. A necessary field
a: MediaButton | undefinedalso becomesval a: MediaButton?. I confirmed this with the same probe. deriveButtonSlotsis already correct for this design. Only the types and the documents change.- TypeScript rejects an incomplete layout. The
favorites.mdexample becomes a compile error, not a defect that a user finds in a car.
The app can still get the capability defaults. It omits remoteButtonLayout, or it sets remoteButtonLayout to null. This keeps one clear switch for the derivation.
Why not a per-field merge
A merge is possible, but I do not recommend it. The derived value is not a constant. The default for back is skip-to-previous, or jump-backward if skip is off, or nothing if both are off. Thus a layout of { forward: 'jump-forward' } gives a different left button for different Capabilities. A change to an unrelated Capability then changes a position that the app did not set. The app cannot see the result at the point of the call.
2. Small items
Give the jump icons a line in the documentation.
Media3 has an icon with a number for 5, 10, 15 and 30 seconds. All other intervals get an arrow with no number. The app cannot see this rule. Please add one line to forwardJumpInterval and to backwardJumpInterval:
On Android, an interval of 5, 10, 15 or 30 seconds gets an icon with that number. Other intervals get an icon without a number.
Do not limit the permitted values. The interval also sets the seek increment, and other values are correct product decisions.
With this rule in the documentation, use an exact comparison in jumpForwardIcon and jumpBackwardIcon. roundToInt() gives the 15-second icon to an interval of 14.6 seconds. The icon then shows a number that does not agree with the seek.
One word in the doc comment.
The doc comment says that a head unit with a spare slot "will promote" the first overflow entry onto the main row. Each head unit decides this for itself. Change will to may.
Add the new terms to the glossary.
CONTEXT.md has no entry for the button layout or for the slots. This PR renames the Kotlin Control enum to Capability, which agrees with the glossary. Please add the two new terms also. CLAUDE.md says that names follow CONTEXT.md.
3. Confirmation of the two publish calls
I examined the Media3 1.10.1 source to check the comment on publishButtonPreferences. The comment is correct.
MediaSessionImpl.setMediaButtonPreferences(List) calls sessionLegacyStub.setPlatformMediaButtonPreferences. It does not call updateLegacySessionPlaybackState. Only the controller overload calls it. And setPlatformMediaButtonPreferences sends new extras only when a reservation flag changes. Thus a layout change that keeps the flags — a new order, or a change of the favorite icon — does not reach the platform PlaybackState. The second call is necessary.
|
🎉 This PR is included in version 0.2.0-next.5 🎉 The release is available on: Your semantic-release bot 📦🚀 |
No description provided.