tedsalmon / BlueBus

A Bluetooth module for vehicles equipped with I-Bus
Other
168 stars 41 forks source link

pressing MFL next track switches BMBT to Bluetooth screen #147

Closed sleuth255 closed 1 year ago

sleuth255 commented 1 year ago

v1.3.0: Display now switches over to the BlueBus screen and stays there when MFL track buttons are used. Kind of a pita if you have the Navigation display up. Maybe this is actually an intended feature and it would be cool if only the display would automatically switch back to the navigation display again. But it doesn't and you actually have to press the "switch" button twice to get the navigation screen back.

sleuth255 commented 1 year ago

So after looking at a few dumps, I can see that this is definitely intended to be a feature. Only the track data is updated in v1.1.18. So I guess put me in as a vote against this. I really only want to see the next track metadata if I'm interested in it. Like if a song that I'm not familiar with plays. In v1.1.18 I would hit the "switch" button if I wanted this. Otherwise, I'm more happy to have the OBD focus remain where it is. Consider this: I'm setting the date/time via the OBD and don't like the track that comes up, so I hit next on my MFL. Very aggravating what happens in v1.3.0 vs. v1.1.18.

nabucho commented 1 year ago

You have a point, it should not force the music display on you. Which "dump" made you think it was intentional? I have been working on this recently, so I may have introduced the regression, but definitely not intentionally. I know I am using the Navi myself but not usually skip songs, but as you brought it to attention, will add this to test suite as well.

sleuth255 commented 1 year ago

Dumps:

Here's the I-Bus sequence on v1.1.18:

[186877] DEBUG: IBus: RX[6]: 50 04 68 3B 01 06 [186997] DEBUG: IBus: RX[25]: 68 17 80 23 41 30 20 20 20 20 20 03 43 44 43 20 31 2D 30 31 04 20 20 20 D3 [187012] DEBUG: IBus: RX[6]: 80 04 68 1B 01 F6 [187021] DEBUG: IBus: RX[6]: 50 04 68 3B 21 26 [187148] DEBUG: IBus: RX[7]: 68 05 18 38 0A 00 47 [187175] DEBUG: IBus: RX[16]: 18 0E 68 39 02 89 00 01 00 01 01 00 01 01 01 CC [SELF] [187546] DEBUG: IBus: RX[14]: 80 0C E7 24 01 00 31 32 3A 30 31 50 4D 6B [187579] DEBUG: IBus: RX[8]: 7F 06 C8 A9 03 30 30 1B [187668] DEBUG: IBus: RX[8]: 7F 06 C8 A9 0A 30 30 12 [187861] DEBUG: IBus: RX[25]: 68 17 80 23 41 30 20 20 20 20 20 03 43 44 43 20 31 2D 30 31 04 20 20 20 D3 [187880] DEBUG: IBus: RX[6]: 80 04 68 1B 01 F6 [188243] DEBUG: IBus: RX[22]: 7F 14 C8 A2 01 00 43 00 29 60 00 88 06 22 31 02 85 00 18 01 00 09 [188287] DEBUG: IBus: RX[37]: 7F 23 C8 A4 00 01 4E 45 57 20 42 45 52 4C 49 4E 2C 20 57 49 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 41 [188330] DEBUG: IBus: RX[37]: 7F 23 C8 A4 00 02 53 20 56 41 4C 4C 45 59 20 53 50 52 49 4E 47 20 43 54 3B 00 00 00 00 00 00 00 00 00 00 00 77 [188608] DEBUG: IBus: RX[27]: 68 19 3B A5 63 01 41 53 68 6F 6F 74 20 48 69 67 68 2C 20 41 69 6D 20 4C 6F 77 B0 [SELF] [188621] DEBUG: IBus: RX[11]: 68 09 3B A5 63 01 42 59 65 73 90 [SELF] [188631] DEBUG: IBus: RX[21]: 68 13 3B A5 63 01 43 42 69 67 20 47 65 6E 65 72 61 74 6F 72 FB [SELF] [188643] DEBUG: IBus: RX[28]: 68 1A 3B A5 63 01 45 54 65 6D 70 3A 20 4F 3A 31 39 34 2C 20 43 3A 33 39 2C 20 41 86 [SELF] [188655] DEBUG: IBus: RX[14]: 68 0C 3B A5 63 15 45 3A 2B 34 31 B0 46 2B [SELF] [188665] DEBUG: IBus: RX[8]: 68 06 3B A5 63 01 00 92 [SELF] [188672] DEBUG: IBus: RX[7]: 3B 05 68 22 00 05 71 [188987] DEBUG: IBus: RX[13]: 7F 0B 80 1F 40 18 01 23 00 04 20 03 B6 [191690] DEBUG: IBus: RX[7]: E8 05 D0 59 11 02 77 [191854] DEBUG: IBus: RX[5]: F0 03 68 01 9A [191865] DEBUG: IBus: RX[6]: 68 04 BF 02 00 D1

and here's the I-Bus sequence on v1.3.0:

[111531] DEBUG: IBus: RX[6]: 50 04 68 3B 01 06 [111695] DEBUG: IBus: RX[6]: 50 04 68 3B 21 26 [111826] DEBUG: IBus: RX[7]: 68 05 18 38 0A 00 47 [111853] DEBUG: IBus: RX[16]: 18 0E 68 39 02 89 00 01 00 01 01 00 01 01 01 CC [SELF] [112578] DEBUG: IBus: RX[25]: 68 17 80 23 41 30 20 20 20 20 20 03 43 44 43 20 31 2D 30 31 04 20 20 20 D3 2D 30 31 04 20 20 20 D3 [112629] DEBUG: IBus: RX[17]: 68 0F 3B 21 62 01 40 42 6C 75 65 74 6F 6F 74 68 08 [SELF] [112640] DEBUG: IBus: RX[8]: 68 06 3B A5 62 01 00 93 [SELF] [112649] DEBUG: IBus: RX[8]: 68 06 3B A5 63 01 00 92 [SELF] [112658] DEBUG: IBus: RX[6]: 80 04 68 1B 01 F6 [112716] DEBUG: IBus: RX[7]: 3B 05 68 22 00 00 74 [112727] DEBUG: IBus: RX[7]: 3B 05 68 22 00 00 74 [112843] DEBUG: IBus: RX[17]: 68 0F 3B 21 62 01 40 42 6C 75 65 74 6F 6F 74 68 08 [SELF] [112854] DEBUG: IBus: RX[8]: 68 06 3B A5 62 01 00 93 [SELF] [112864] DEBUG: IBus: RX[22]: 68 14 3B A5 62 01 06 4B 65 76 69 6E 27 73 20 69 50 68 6F 6E 65 99 [SELF] [112877] DEBUG: IBus: RX[10]: 68 08 3B A5 62 01 02 3E 20 81 [SELF] [112887] DEBUG: IBus: RX[12]: 68 0A 3B A5 62 01 01 20 20 20 20 9E [SELF] [112897] DEBUG: IBus: RX[8]: 68 06 3B A5 62 01 00 93 [SELF] [112906] DEBUG: IBus: RX[7]: 3B 05 68 22 00 00 74 [112914] DEBUG: IBus: RX[7]: 3B 05 68 22 00 00 74 [112924] DEBUG: IBus: RX[7]: 3B 05 68 22 00 00 74 [112935] DEBUG: IBus: RX[7]: 3B 05 68 22 00 00 74 [112946] DEBUG: IBus: RX[7]: 3B 05 68 22 00 00 74 [113287] DEBUG: IBus: RX[16]: 68 0E 3B A5 63 01 41 49 27 6D 20 46 72 65 65 CC [SELF] [113299] DEBUG: IBus: RX[15]: 68 0D 3B A5 63 01 42 54 68 65 20 57 68 6F F2 [SELF] [113311] DEBUG: IBus: RX[28]: 68 1A 3B A5 63 01 43 54 6F 6D 6D 79 20 28 44 65 6C 75 78 65 20 45 64 69 74 69 6F B8 [SELF] [113325] DEBUG: IBus: RX[10]: 68 08 3B A5 63 15 43 6E 29 8C [SELF] [113335] DEBUG: IBus: RX[26]: 68 18 3B A5 63 01 45 4F 3A 31 38 31 2C 43 3A 34 35 2C 41 3A 2B 34 31 B0 46 5F [SELF] [113349] DEBUG: IBus: RX[8]: 68 06 3B A5 63 01 00 92 [SELF] [113358] DEBUG: IBus: RX[7]: 3B 05 68 22 00 05 71 [114774] DEBUG: IBus: RX[7]: E8 05 D0 59 11 02 77 [114873] DEBUG: IBus: RX[5]: F0 03 68 01 9A [114885] DEBUG: IBus: RX[6]: 68 04 BF 02 00 D1 [115207] DEBUG: IBus: RX[5]: 68 03 6A 01 00

sleuth255 commented 1 year ago

Pressing the next track button on the OBD also switches the display, but only momentarily. I'm guessing that it must have something to do with the nature of the trigger in that bluebus isn't responding to a RAD - CDC request to change tracks in this instance.

tedsalmon commented 1 year ago

This is the offending commit / line of code:

https://github.com/tedsalmon/BlueBus/commit/e9a2f256ef6703fa88cad95ecadc62a70a55a662#diff-17c37c08dd5d6a121bb483a5dd6d2acc342fbcfb78dd255414ac46d9dd19115aR505

sleuth255 commented 1 year ago

Yup. That fixed it. Many thanks!

tedsalmon commented 1 year ago

Yup. That fixed it. Many thanks!

To explain further, this code was added so that CD54 (E46 Professional Radios) screen updates could be "seen" consistently and the BlueBus could override the display quickly. I simply forgot that the same message was used by the Nav radio to update the high OBC. The real fix is to return here: https://github.com/tedsalmon/BlueBus/blob/master/firmware/application/ui/bmbt.c#L2043 if the destination is set to the IKE :)

sleuth255 commented 1 year ago

Agree. If the BMBT handler isn't interested in IKE destination messages then it should just toss them. ie:

if (pkt[IBUS_PKT_DST] == IBUS_DEVICE_IKE) return;

On my copy of v1.3.1, I put this change into bmbt.c as the first test in BMBTRADUpdateMainArea and it works as advertised.

sleuth255 commented 1 year ago

Thanks for fixing this in 1.3.2.