len0rd/rockbox
1
0
Fork 1
mirror of https://github.com/Rockbox/rockbox.git synced 2026-07-10 13:29:52 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions

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

Browse source 
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
This commit is contained in:
Le Khanh Binh 2026-06-11 18:24:38 +07:00
parent c3ad489738
commit d00a53a1f0
3 changed files with 114 additions and 63 deletions
Download patch file Download diff file Expand all files Collapse all files

109
apps/iap/iap-core.c
View file

@ -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)
{
queue_post(&iap_queue, IAP_EV_MSG_RCVD, 0);
sleep(1);
return;
}
/* 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)));
#endif
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;
#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;
#if CONFIG_TUNER
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
/* 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.
*/
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);
while (iap_rxpayload != iap_rxstart)
{
/* 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;
}
/* poke the poweroff timer */
reset_poweroff_timer();
/* 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)));
#endif
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;
#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;
#if CONFIG_TUNER
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();
}
}
int remote_control_rx(void)

67
apps/iap/iap-lingo2.c
View file

@ -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 */

1
docs/CREDITS
View file

@ -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