iap: Fix dead remote at boot & ghost clicks on held buttons

commit d00a53a1f0df96fbd45ca204ff023eb5b1f10ae5
Author: Le Khanh Binh <[email protected]>
Date:   Thu Jun 11 18:24:38 2026 +0700

    iap: Fix dead remote at boot & ghost clicks on held buttons
    
    Fix FS#13832
    If the remote is already plugged in when Rockbox boots, it stays dead
    until you replug it. These remotes only send their identify packet
    once at power-up, so we never see it and then throw away all button
    events because lingo 0x02 was never negotiated. The OF doesn't care,
    so accept ContextButtonStatus from unidentified devices too. Stray
    bytes received before iap_setup() also made iap_getc() return false,
    which the serial driver takes as "sync found" and locks autobaud onto
    the wrong bitrate.
    
    The ghost clicks come from the repeatbtn handshake. A held button
    resends its down event every 30-100ms and each one re-armed
    iap_repeatbtn, which stalls handling of the following packet. So
    during a long press packets pile up, the event queue overflows, and
    the leftovers replay as keypresses after you let go. Skipping fires
    on button release which is why next/prev showed it the worst. Now
    repeatbtn is only re-armed when the button state actually changes,
    and iap_handlepkt() drains whatever complete packets are in the RX
    buffer rather than relying on one queued event per packet.
    
    While at it, fix two more things found along the way: the button
    state is now written in one go (the temporary BUTTON_NONE could be
    picked up by the button tick mid-update), and iap_rxlen is
    decremented when a frame is received so the overflow check actually
    does something.
    
    Tested on ipodmini2g with an A1018 remote.
    
    Change-Id: Ie18512e47d642fe6957fc010eaf79b6d2af4c070

diff --git a/apps/iap/iap-core.c b/apps/iap/iap-core.c
index 74743fef03..0b4ad13477 100644
--- a/apps/iap/iap-core.c
+++ b/apps/iap/iap-core.c
@@ -373,6 +373,10 @@ static void iap_thread(void)
             case IAP_EV_TICK:
             {
                 iap_periodic();
+                /* pick up packets whose IAP_EV_MSG_RCVD event was lost
+                   to a queue overflow, they would otherwise sit in the
+                   RX buffer until the next packet arrives */
+                iap_handlepkt();
                 break;
             }
 
@@ -597,8 +601,12 @@ bool iap_getc(IF_IAP_MP(int port,) const unsigned char x)
     struct state_t *s = &frame_state;
     static long pkt_timeout;
 
+    /* Report "still hunting" while IAP is not set up yet, otherwise
+     * the serial driver would lock its autobaud detection onto the
+     * default bitrate before any real traffic was seen.
+     */
     if (!iap_setupflag)
-        return false;
+        return true;
 
     /* Check the time since the last packet arrived. */
     if ((s->state != ST_SYNC) && TIME_AFTER(current_tick, pkt_timeout)) {
@@ -685,6 +693,7 @@ bool iap_getc(IF_IAP_MP(int port,) const unsigned char x)
         s->check += x;
         if ((s->check & 0xFF) == 0) {
             /* done, received a valid frame */
+            iap_rxlen -= (s->len + 2);
             iap_rxpayload = iap_rxnext;
             queue_post(&iap_queue, IAP_EV_MSG_RCVD, 0);
         } else {
@@ -1287,53 +1296,63 @@ void iap_handlepkt(void)
     int length;
 
     if(!iap_setupflag) return;
+    if(!iap_running) return;
 
-    /* if we are waiting for a remote button to go out,
-       delay the handling of the new packet */
-    if(iap_repeatbtn)
+    /* The number of queued IAP_EV_MSG_RCVD events does not reliably
+     * match the number of complete packets in the RX buffer, since
+     * the event queue can overflow during input bursts. Drain every
+     * complete packet instead, they occupy the buffer from
+     * iap_rxstart up to iap_rxpayload.
+     */
+    while (iap_rxpayload != iap_rxstart)
     {
-        queue_post(&iap_queue, IAP_EV_MSG_RCVD, 0);
-        sleep(1);
-        return;
-    }
+        /* if we are waiting for a remote button to go out,
+           delay the handling of the new packet */
+        if(iap_repeatbtn)
+        {
+            queue_post(&iap_queue, IAP_EV_MSG_RCVD, 0);
+            sleep(1);
+            return;
+        }
 
-    /* handle command by mode */
-    length = get_u16(iap_rxstart);
+        /* handle command by mode */
+        length = get_u16(iap_rxstart);
 #if defined(LOGF_ENABLE) && defined(ROCKBOX_HAS_LOGF)
-    logf("R: %s", hexstring(iap_rxstart+2, (length)));
+        logf("R: %s", hexstring(iap_rxstart+2, (length)));
 #endif
 
-    if (length != 0) {
-        unsigned char mode = *(iap_rxstart+2);
-        switch (mode) {
-        case 0: iap_handlepkt_mode0(length, iap_rxstart+2); break;
+        if (length != 0) {
+            unsigned char mode = *(iap_rxstart+2);
+            switch (mode) {
+            case 0: iap_handlepkt_mode0(length, iap_rxstart+2); break;
 #ifdef HAVE_LINE_REC
-        case 1: iap_handlepkt_mode1(length, iap_rxstart+2); break;
+            case 1: iap_handlepkt_mode1(length, iap_rxstart+2); break;
 #endif
-        case 2: iap_handlepkt_mode2(length, iap_rxstart+2); break;
-        case 3: iap_handlepkt_mode3(length, iap_rxstart+2); break;
-        case 4: iap_handlepkt_mode4(length, iap_rxstart+2); break;
-        case 5: iap_handlepkt_mode5(length, iap_rxstart+2); break;
+            case 2: iap_handlepkt_mode2(length, iap_rxstart+2); break;
+            case 3: iap_handlepkt_mode3(length, iap_rxstart+2); break;
+            case 4: iap_handlepkt_mode4(length, iap_rxstart+2); break;
+            case 5: iap_handlepkt_mode5(length, iap_rxstart+2); break;
 #if CONFIG_TUNER
-        case 7: iap_handlepkt_mode7(length, iap_rxstart+2); break;
+            case 7: iap_handlepkt_mode7(length, iap_rxstart+2); break;
 #endif
+            }
         }
-    }
 
-    /* Remove the handled packet from the RX buffer
-     * This needs to be done with interrupts disabled, to make
-     * sure the buffer and the pointers into it are handled
-     * cleanly
-     */
-    level = disable_irq_save();
-    memmove(iap_rxstart, iap_rxstart+(length+2), (RX_BUFLEN+2)-(length+2));
-    iap_rxnext -= (length+2);
-    iap_rxpayload -= (length+2);
-    iap_rxlen += (length+2);
-    restore_irq(level);
-
-    /* poke the poweroff timer */
-    reset_poweroff_timer();
+        /* Remove the handled packet from the RX buffer
+         * This needs to be done with interrupts disabled, to make
+         * sure the buffer and the pointers into it are handled
+         * cleanly
+         */
+        level = disable_irq_save();
+        memmove(iap_rxstart, iap_rxstart+(length+2), (RX_BUFLEN+2)-(length+2));
+        iap_rxnext -= (length+2);
+        iap_rxpayload -= (length+2);
+        iap_rxlen += (length+2);
+        restore_irq(level);
+
+        /* poke the poweroff timer */
+        reset_poweroff_timer();
+    }
 }
 
 int remote_control_rx(void)
diff --git a/apps/iap/iap-lingo2.c b/apps/iap/iap-lingo2.c
index 31a49c4789..58c79504a5 100644
--- a/apps/iap/iap-lingo2.c
+++ b/apps/iap/iap-lingo2.c
@@ -71,8 +71,15 @@ void iap_handlepkt_mode2(const unsigned int len, const unsigned char *buf)
      */
     CHECKLEN(3);
 
-    /* Lingo 0x02 must have been negotiated */
-    if (!DEVICE_LINGO_SUPPORTED(0x02)) {
+    /* Lingo 0x02 must have been negotiated, except for
+     * ContextButtonStatus (0x00): simple remotes like the Apple A1018
+     * identify only once at power-up. If the remote was already
+     * powered before Rockbox started (e.g. plugged in at boot) that
+     * identification is never seen, and rejecting the button events
+     * would leave the remote dead until it is replugged. The OF
+     * accepts these events without identification, so do the same.
+     */
+    if ((cmd != 0x00) && !DEVICE_LINGO_SUPPORTED(0x02)) {
         cmd_ack(cmd, IAP_ACK_BAD_PARAM);
         return;
     }
@@ -95,14 +102,13 @@ void iap_handlepkt_mode2(const unsigned int len, const unsigned char *buf)
          */
         case 0x00:
         {
-            iap_remotebtn = BUTTON_NONE;
-            iap_timeoutbtn = 0;
+            unsigned long btn = BUTTON_NONE;
 
             if(buf[2] != 0)
             {
                 if(buf[2] & 1)
                 {
-                    REMOTE_BUTTON(BUTTON_RC_PLAY);
+                    btn |= BUTTON_RC_PLAY;
 #if CONFIG_TUNER
                     if (radio_present == 1) {
                         if (remote_mute == 0) {
@@ -117,20 +123,20 @@ void iap_handlepkt_mode2(const unsigned int len, const unsigned char *buf)
 #endif
                 }
                 if(buf[2] & 2)
-                    REMOTE_BUTTON(BUTTON_RC_VOL_UP);
+                    btn |= BUTTON_RC_VOL_UP;
                 if(buf[2] & 4)
-                    REMOTE_BUTTON(BUTTON_RC_VOL_DOWN);
+                    btn |= BUTTON_RC_VOL_DOWN;
                 if(buf[2] & 8)
-                    REMOTE_BUTTON(BUTTON_RC_RIGHT);
+                    btn |= BUTTON_RC_RIGHT;
                 if(buf[2] & 16)
-                    REMOTE_BUTTON(BUTTON_RC_LEFT);
+                    btn |= BUTTON_RC_LEFT;
             }
             else if(len >= 4 && buf[3] != 0)
             {
                 if(buf[3] & 1) /* play */
                 {
                     if (audio_status() != AUDIO_STATUS_PLAY)
-                        REMOTE_BUTTON(BUTTON_RC_PLAY);
+                        btn |= BUTTON_RC_PLAY;
 #if CONFIG_TUNER
                     if (radio_present == 1) {
                         tuner_set(RADIO_MUTE,0);
@@ -140,7 +146,7 @@ void iap_handlepkt_mode2(const unsigned int len, const unsigned char *buf)
                 if(buf[3] & 2) /* pause */
                 {
                     if (audio_status() == AUDIO_STATUS_PLAY)
-                        REMOTE_BUTTON(BUTTON_RC_PLAY);
+                        btn |= BUTTON_RC_PLAY;
 #if CONFIG_TUNER
                     if (radio_present == 1) {
                         tuner_set(RADIO_MUTE,1);
@@ -179,24 +185,49 @@ void iap_handlepkt_mode2(const unsigned int len, const unsigned char *buf)
                 if (buf[4] & 0x04)
                 {
                     if (audio_status() == AUDIO_STATUS_PLAY)
-                        REMOTE_BUTTON(BUTTON_RC_PLAY);
+                        btn |= BUTTON_RC_PLAY;
                 }
 
                 if(buf[4] & 16) /* ffwd */
-                    REMOTE_BUTTON(BUTTON_RC_RIGHT);
+                    btn |= BUTTON_RC_RIGHT;
                 if(buf[4] & 32) /* frwd */
-                    REMOTE_BUTTON(BUTTON_RC_LEFT);
+                    btn |= BUTTON_RC_LEFT;
                 if(buf[4] & 64) /* menu */
-                    REMOTE_BUTTON(BUTTON_RC_MENU);
+                    btn |= BUTTON_RC_MENU;
                 if(buf[4] & 128) /* select */
-                    REMOTE_BUTTON(BUTTON_RC_SELECT);
+                    btn |= BUTTON_RC_SELECT;
             }
             else if(len >= 6 && buf[5] != 0)
             {
                 if(buf[5] & 1) /* up */
-                    REMOTE_BUTTON(BUTTON_RC_UP);
+                    btn |= BUTTON_RC_UP;
                 if (buf[5] & 2) /* down */
-                    REMOTE_BUTTON(BUTTON_RC_DOWN);
+                    btn |= BUTTON_RC_DOWN;
+            }
+
+            /* Commit the new button state in one go. The button driver
+             * reads iap_remotebtn from interrupt context, a transient
+             * BUTTON_NONE between two repeated button-down events would
+             * register as a spurious release/press pair.
+             */
+            if (btn != BUTTON_NONE)
+            {
+                /* Only re-arm iap_repeatbtn when the state changes. A
+                 * held button repeats its down event every 30-100ms,
+                 * re-arming on every repeat makes iap_handlepkt()
+                 * delay each following packet, so packets back up
+                 * during the hold and stale events replay as phantom
+                 * keypresses after the release.
+                 */
+                if (btn != iap_remotebtn)
+                    iap_repeatbtn = 2;
+                iap_remotebtn = btn;
+                iap_timeoutbtn = 3;
+            }
+            else
+            {
+                iap_remotebtn = BUTTON_NONE;
+                iap_timeoutbtn = 0;
             }
 
             /* power on released */
diff --git a/docs/CREDITS b/docs/CREDITS
index 0ec830bc14..d1ac0cfdc7 100644
--- a/docs/CREDITS
+++ b/docs/CREDITS
@@ -762,6 +762,7 @@ Sergey Puskov
 Eivind Ødegård
 Teun van Dalen
 Adam N. Burke
+Lê Khánh Bình
 
 The libmad team
 The wavpack team
-- 
rockbox-cvs mailing list
[email protected]
https://lists.haxx.se/mailman/listinfo/rockbox-cvs