Skip to content

Keep the selected CAN line speed when switching screens - #1886

Open
lennardm wants to merge 1 commit into
cedricp:masterfrom
lennardm:fix/can-line-combo-reset
Open

lennardm wants to merge 1 commit into
cedricp:masterfrom
lennardm:fix/can-line-combo-reset

Conversation

@lennardm

@lennardm lennardm commented Oct 5, 2026 •

Copy link
Copy Markdown
Contributor

Problem
Some cars run their diagnostic CAN bus at 250 kbit/s instead of 500 kbit/s. For example, on a Renault Espace IV ph2 the bus at the OBD socket is at 250K, and "CAN Line 1 Auto" connects at 500K, so every request fails with CAN ERROR. The workaround is to select "CAN Line 1@250K" in the toolbar, but that only lasts until you open another screen:

  • every screen change resets the CAN line selector to "CAN Line 1 Auto";
  • the ECU is reconnected at 500K, and communication stops again;
  • the log shows the ECU being reconnected many times in a row.
    So in practice it's impossible to work with an ECU that needs a manually selected CAN speed.

Cause
changeScreen() calls set_can_combo(), which clears and refills the combo box. That resets the selection to index 0 ("Auto") and fires currentIndexChanged, which reconnects at that speed. The function also disconnects the wrong signal (clicked instead of currentIndexChanged) before connecting it again. Each screen change therefore adds another connection, and one change ends up triggering several reconnects.

Fix
set_can_combo() now:

  • remembers the selected index,
  • rebuilds the combo with signals blocked,
  • disconnects currentIndexChanged before reconnecting it,
  • restores the previous selection.

Changing screens no longer changes the CAN speed or triggers reconnects. Choosing a different speed still reconnects once, as before.

Tested on a Renault Grand Espace IV ph2 (EDC16CP33 over CAN at 250K, vLinker FS): the selection stays on "CAN Line 1@250K" across screen changes, with a single reconnect when selecting it.

Goes together with PR #1887

changeScreen() rebuilds the CAN line combo on every screen change. The
rebuild reset the selection to "CAN Line 1 Auto", which reconnected the
ECU at 500K, and it connected currentIndexChanged again each time while
only disconnecting the (unused) clicked signal. The extra connections
made one change trigger several reconnects in a row.

Remember the selected index, rebuild the combo with signals blocked,
disconnect currentIndexChanged before reconnecting it, and restore the
selection. Needed for ECUs that only answer at 250K, e.g. the EDC16CP33
on an Espace IV ph2 (Auto selects 500K there).

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@Furtif

Furtif commented Oct 5, 2026

Copy link
Copy Markdown
Collaborator

I'm not sure if this is correct with the merge of fix/issue_1888 and also in the pull request #1887

@lennardm

lennardm commented Oct 6, 2026 •

Copy link
Copy Markdown
Contributor Author

I'm not sure if this is correct with the merge of fix/issue_1888 and also in the pull request #1887

#1889 (fix for #1888) and #1887 fix different things:

Reconnects still happen after #1889, for example:

  1. when the user changes the CAN line in the toolbar, which is still needed for ECU files without the baudrate hint, where "Auto" still means 500K;
  2. after an automatic reconnect when the adapter connection was lost (sendElm() → reconnect_elm() → initELM());
  3. on every screen change, because set_can_combo() rebuilds the combo and fires currentIndexChanged → setCanLine() → initELM(), which is Keep the selected CAN line speed when switching screens #1886. Fix/issue 1888 can auto 250k detection #1889 doesn't touch set_can_combo(); on current master every screen change still rebuilds the combo, resets a manual CAN line choice to Auto, and reconnects, because currentIndexChanged fires on clear()/addItem() and the wrong signal (clicked) is disconnected.

So both #1886 and #1887 are still needed. #1886 and #1887 are independent of #1889 and of each other: #1886 stops the screen change from resetting the CAN line and reconnecting, and #1887 makes sure any reconnect that does happen reopens the session.

@Furtif

Furtif commented Oct 6, 2026

Copy link
Copy Markdown
Collaborator

ok 👍

@Furtif

Furtif commented Oct 6, 2026

Copy link
Copy Markdown
Collaborator

So this is ready to merge?

@KarelSvo

KarelSvo commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

doesn't touch set_can_combo(); on current master every screen change still rebuilds the combo, resets a manual CAN line choice to Auto, and reconnects, because currentIndexChanged fires on clear()/addItem() and the wrong signal (clicked) is disconnected.

Do you have an ELM log file showing the connection restarting while selecting different screens within an ECU?
I don't recall ever seeing anything like that—at least not with the original ddt4all versions from Cedric, which don't contain any AI-distorted code.
If the protocol is automatically identified upon startup, there is no need to manually select protocols later.
The "Can Line" switch was originally intended for manually switching the CAN 2 interface on units with two CAN modules via STP 53.
"brp" is a switch found in the Clip database. Checking the Clip vehicle files reveals that there is no vehicle with CAN 2 where brp=1.
Therefore, the canline (STP 53) is always set to STPBR 500000, rendering the switches for STPBR 250000 and STPBR 125000 superfluous.

pic

@lennardm

lennardm commented Oct 7, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the detailed explanation and the CLIP excerpt. Agreed on CAN2: if there's no CLIP vehicle with brp=1 on the second network, the "CAN Line 2@250K/125K" entries are indeed superfluous. (Your excerpt also nicely shows brp=1 on the 6/14 network, which matches my Espace IV ph2 at 250K.) This PR doesn't change which entries exist, though; it's only about what happens to the selection when you change screens.

ELM log (unmodified set_can_combo(), EDC16CP33 W98 over CAN). One screen change at 15:36:02 gives 12 reconnects in a row, back at 500K:

# [10/04/26 15:36:02.570] Init CAN
# Connect to: [EDC16CP33 - W98 - V4 - (Diag On CAN)] Addr: 15
[15:36:02.875] Request: AT SP 6
# [10/04/26 15:36:02.970] Init CAN
# Connect to: [EDC16CP33 - W98 - V4 - (Diag On CAN)] Addr: 15
[15:36:03.274] Request: AT SP 6
... (repeats every ~0.4 s until 15:36:07.275, 12x in total)

After selecting @250K again at 15:36:11, it reconnects about 12 times with AT SP 8.

Reproduction without a car, using the set_can_combo() from current master in a QComboBox, counting calls to changecanspeed() (-> setCanLine() -> initELM()):

Action Reconnects Selection afterwards
select @250K 1 CAN Line 1@250K
screen change 1 2 (index -1, then 0) CAN Line 1 Auto
screen change 2 4 Auto
screen change 3 6 Auto

clear() and the first addItem() both emit currentIndexChanged, and since clicked is disconnected instead of currentIndexChanged, a new connection is added on every rebuild. This also happens in Auto mode if the selection is never touched; it's just less visible when Auto happens to choose the right speed.

This behaviour has been there since set_can_combo() was introduced in 6acccf5 (2020); changeScreen() calls it on every screen change. The PR only preserves the selection and stops the extra reconnects.

@KarelSvo

KarelSvo commented Oct 8, 2026 •

Copy link
Copy Markdown
Contributor

set_can_combo shouldn't actually activate if the interface check results in options.opt_stn_basic = False. STN interfaces without a second CAN module do not require a CAN 2 switch.

@lennardm

lennardm commented Oct 8, 2026

Copy link
Copy Markdown
Contributor Author

set_can_combo shouldn't actually activate if the interface check results in options.opt_stn_basic = False. STN interfaces without a second CAN module do not require a CAN 2 switch.

That would be a separate change from this PR, and it wouldn't remove the problem: on STN adapters the combo stays, and my vLinker FS is one (STN1170 v4.3.2, so opt_stn_basic = True). The combo is still rebuilt on every screen change and the selection resets.

Also, the "CAN Line 1 Auto/@500K/@250K" entries aren't CAN2: they switch the normal 6/14 line between AT SP 6 and AT SP 8, which works on any ELM327. Only the "CAN Line 2@..." entries are CAN2, and those are already only added for ELS adapters. Hiding the whole combo for non-STN adapters would remove the manual 250K option for ECU files that have no brp hint.

If the maintainers want to restrict or rename the CAN line options, I'd suggest a separate issue/PR for that. This one only keeps the selection across screen changes and stops the repeated reconnects.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants