feat: support legacy ICOM (A1/A2) over SLP on ISTA 4.60+ - #92
Merged
Conversation
ISTA 4.60 rewrote SLP device dispatch to only recognize DevType=="ICOM-Next";
legacy units reporting DevType=="ICOM" now fall through and get rejected
("newDevice is null or invalid"). Normalize DevType right after ParseAttrList
on 4.60+ so the existing ICOM-Next dispatch branch is taken; keep the prior
ReplaceDeviceType approach for versions below 4.60.
NormalizeDevType imported Dictionary.get_Item/set_Item via typeof(), which under the net10.0 patcher binds to System.Private.CoreLib. That assembly does not exist on ISTA's .NET Framework runtime, so the patched ScanDeviceFromAttrList failed to resolve at JIT and broke VCI detection (including genuine ICOM-Next). Reuse the existing mscorlib get_Item/op_Equality operands and build set_Item on the same declaring type.
Differentiating variant: instead of remapping DevType in the dictionary, widen the ICOM-Next dispatch condition to also accept DevType ICOM/ICOM A1/ICOM A2, mask their DevTypeExt to ICOM_Next_A, and force the raw reported State only for A1/A2. DevType is never rewritten, so A1/A2 stay distinguishable from a genuine ICOM-Next at the State block; genuine ICOM-Next keeps the upstream dispatch, firmware check and State intact (no firmware patch needed). Dictionary members are bound to the target corlib (mscorlib), not System.Private.CoreLib.
Real legacy ICOM A1/A2 report DevType="ICOM" with DevTypeExt="ICOM_A1"/"ICOM_A2"; only ICOM-Next reports DevType="ICOM-Next". The previous variant keyed on a non-existent DevType=="ICOM A1"/"ICOM A2" (dead branches) and overwrote DevTypeExt with "ICOM_Next_A", erasing the A1/A2 identity. Now: widen dispatch to also accept DevType=="ICOM"; leave DevTypeExt untouched; force raw State only when DevTypeExt is "ICOM_A1"/"ICOM_A2", so genuine ICOM-Next (DevTypeExt="ICOM_Next_A") keeps its upstream firmware/State. Reuses existing get_Item/ContainsKey/op_Equality operands.
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 39 |
| Duplication | 0 |
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
…ed helpers Split the oversized RewriteIcomNextDispatch into WidenIcomDispatch and KeepRawStateForLegacyIcom, shrinking PatchSLP and addressing the CodeFactor complex-method report. Emitted IL is unchanged.
OrpheeGT
force-pushed
the
feat/4.60-icom-next-patch
branch
from
July 2, 2026 16:59
742be35 to
1c05ed6
Compare
cmitrou
added a commit
to cmitrou/ISTA-Patcher
that referenced
this pull request
Jul 22, 2026
* feat: replace bash scripts with Python equivalents for copyright checks and test coverage * feat: refactor Rheingold DatabaseProvider and LicenseManagement Models * feat: improve reportgenerator handling and error reporting in test coverage script * test: enhance test coverage and refactor utility methods * feat: add --enable-ista-voltage option to inject an ISTAVoltageControl.dll (tautcony#88) Hooks into IstaOperation.exe's InitializeServices, loading ISTAVoltageControl.dll at runtime and calling Controller.StartFromIstaOperation to enable KL15/KL30 voltage tracking over OBD/D-CAN or ENET. The injection is idempotent and wrapped in a silent try/catch so it never crashes ISTA. IMPORTANT — two external dependencies are required, neither of which is part of this repository: 1. EdiabasLib (Api64.dll) The DLL is resolved at runtime via the BinPathModifications registry key (HKLM\SOFTWARE\BMWGroup\ISPI\Rheingold\BMW.Rheingold.ISTAGUI .BinPathModifications), falling back to the stock ISTA Ediabas BIN folder. Source: https://github.com/uholeschak/ediabaslib 2. ISTAVoltageControl.dll An external voltage-controller bridge that must be deployed into the folder resolved above before launching ISTA. This patch only installs the bootstrap call; all voltage logic (KL15/KL30 polling, VCI walk) lives inside that DLL. To implement your own, the expected contract is defined in PatchUtils.ISTAVoltage.cs (this repo): a public static method ISTAVoltageControl.Controller.StartFromIstaOperation(object serviceImpl). From there, use EdiabasVoltageBridge.TryRegister() (EdiabasLib) to receive KL15/KL30 samples via callback. If the flag is used but either dependency is missing, the silent try/catch ensures ISTA continues to start normally. * feat: add --enable-skip-battery-demand option (tautcony#89) * feat: block EDGE telemetry egress (Speedlink + OBFCM) in DataNotSend (tautcony#90) * feat: block EDGE telemetry egress (Speedlink + OBFCM) in DataNotSend The IsSend*Forbidden flags do not gate these channels: Logic.SendSpeedlinkDataInBackground and SendObfcmDataToVehicleShadowBackend fire the HTTP POST unconditionally (the forbidden flag only rides inside the payload), so the request still leaves the machine. Empty both senders (DnlibUtils.EmptyingMethod) so the telemetry call never happens. Scope: telemetry only -- Speedlink (session/dealer) + OBFCM (fuel data to Vehicle Shadow). EDGEBattery / EDGEPDI are left functional. To extend to zero EDGE egress, empty EDGEProcessorImpl.SendDataToBackend instead (single chokepoint, blocks all channels). * feat: block SCC vehicle-session upload in DataNotSend SaveCurrentSCCCase -> SCCProcessorImpl.PostVehicleSession runs unconditionally on session close and is not gated by the IsSend*Forbidden flags, posting VIN, vehicle data and diagnosis data (DTCs, technician statements, PDF files) to the SCC backend. Return its own offline sentinel (ServiceUnavailable) so the call never leaves the machine, mirroring PatchEdgeTelemetry. * feat: support legacy ICOM (A1/A2) over SLP on ISTA 4.60+ (tautcony#92) * feat: handle ICOM-Next device dispatch rewrite for ISTA 4.60+ ISTA 4.60 rewrote SLP device dispatch to only recognize DevType=="ICOM-Next"; legacy units reporting DevType=="ICOM" now fall through and get rejected ("newDevice is null or invalid"). Normalize DevType right after ParseAttrList on 4.60+ so the existing ICOM-Next dispatch branch is taken; keep the prior ReplaceDeviceType approach for versions below 4.60. * fix(PatchSLP): bind injected SLP dictionary members to the target corlib NormalizeDevType imported Dictionary.get_Item/set_Item via typeof(), which under the net10.0 patcher binds to System.Private.CoreLib. That assembly does not exist on ISTA's .NET Framework runtime, so the patched ScanDeviceFromAttrList failed to resolve at JIT and broke VCI detection (including genuine ICOM-Next). Reuse the existing mscorlib get_Item/op_Equality operands and build set_Item on the same declaring type. * feat(PatchSLP): A1/A2-aware ICOM dispatch with State gate (>= 4.60) Differentiating variant: instead of remapping DevType in the dictionary, widen the ICOM-Next dispatch condition to also accept DevType ICOM/ICOM A1/ICOM A2, mask their DevTypeExt to ICOM_Next_A, and force the raw reported State only for A1/A2. DevType is never rewritten, so A1/A2 stay distinguishable from a genuine ICOM-Next at the State block; genuine ICOM-Next keeps the upstream dispatch, firmware check and State intact (no firmware patch needed). Dictionary members are bound to the target corlib (mscorlib), not System.Private.CoreLib. * fix(PatchSLP): drive legacy ICOM off DevTypeExt, not DevType Real legacy ICOM A1/A2 report DevType="ICOM" with DevTypeExt="ICOM_A1"/"ICOM_A2"; only ICOM-Next reports DevType="ICOM-Next". The previous variant keyed on a non-existent DevType=="ICOM A1"/"ICOM A2" (dead branches) and overwrote DevTypeExt with "ICOM_Next_A", erasing the A1/A2 identity. Now: widen dispatch to also accept DevType=="ICOM"; leave DevTypeExt untouched; force raw State only when DevTypeExt is "ICOM_A1"/"ICOM_A2", so genuine ICOM-Next (DevTypeExt="ICOM_Next_A") keeps its upstream firmware/State. Reuses existing get_Item/ContainsKey/op_Equality operands. * refactor(PatchSLP): extract dispatch widening and State gate into named helpers Split the oversized RewriteIcomNextDispatch into WidenIcomDispatch and KeepRawStateForLegacyIcom, shrinking PatchSLP and addressing the CodeFactor complex-method report. Emitted IL is unchanged. * fix: scope the ENET real-ECU voltage-check patch to the diagnostic module loader only (tautcony#93) * feat: skip ENET real-ECU voltage check in ModuleBootstrapLoader.doIt() VoltageUtils.CheckVoltageForEthernetConnection (RheingoldProgramming.dll) reads real battery voltage via an ECU service program ("Batteriespannung") on ENET connections, independent of VCI.Kl15/Kl30Voltage. This means PatchISTAVoltageControl's injected voltage never covers it: an ENET session on battery alone can still show a genuine "battery voltage (terminal 30) below threshold value" message during plain diagnostic work, even while ISTA's header displays a healthy injected value. ModuleBootstrapLoader.doIt() -- the diagnostic test-module execution loop, invoked on every module load -- is one of three independent callers of that check. This patch targets only that call site: the leading (short-circuit &&) branch on ModuleLoader.IsModuleLoaderInitialized is turned into an unconditional jump to the same target the false-case already used, so the guarded block (including the voltage check call) is skipped regardless of the flag's real value. Logic.DoInitialElectricalChecks() (one-shot at connection) and the Toyota-only call inside ProgrammingUtils.CheckVehicleProgramingProhibits (a programming-plan gate) are deliberately left untouched -- this patch never affects anything reachable from a coding/flashing session. * docs: add missing <param>/<returns> tags (SA1611/SA1615) PatchModuleBootstrapLoaderEthernetVoltageCheck's XML doc comment was missing the <param name="module"> and <returns> tags flagged by CodeFactor/StyleCop. * feat: implement cancellation token support across commands and utilities * feat: reorganize test namespaces and add global usings for improved structure --------- Co-authored-by: TautCony <i@tautcony.xyz> Co-authored-by: OrpheeGT <OrpheeGT@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ISTA 4.60 rewrote the SLP device dispatch in
SLP.ScanDeviceFromAttrList(RheingoldxVM.dll) to only recognizeDevType == "ICOM-Next". Legacy ICOM units (ICOM-A / A1 / A2) reportDevType == "ICOM"(withDevTypeExt == "ICOM_A1"/"ICOM_A2"), so on 4.60+ they fall through to the ENET check andScanDeviceFromAttrListreturnsnull→ the VCI is never listed. Even when forced through, they are flagged unsupported / firmware-update-required and end up in device State"14"/"15"("Firmware"), which blocks the connection.This PR extends
PatchSLPso legacy ICOM boxes are recognized and usable on 4.60+, while leaving genuine ICOM-Next behavior strictly upstream.What changed (
PatchUtils.PatchSLP, path>= 4.60)DevType == "ICOM"in addition to"ICOM-Next", so legacy units take the existing ICOM-Next branch.DevTypeExtis"ICOM_A1"/"ICOM_A2", bypassing the firmware-update"14"downgrade.DevTypeExtis never rewritten, so the A1/A2 identity is preserved.IsIcomUnsupported → false(unchanged from before) — prevents the unsupported-path blanking of IP/VIN/VciChannels and the"15"State for A1/A2.Genuine ICOM-Next (
DevType="ICOM-Next",DevTypeExt="ICOM_Next_A") does not match the A1/A2 gate, so its dispatch, firmware check and State remain exactly as upstream.Versions < 4.60 keep the original
ReplaceDeviceTypepath, untouched.DevTypeDevTypeExtICOM-NextICOM_Next_AICOMICOM_A1/ICOM_A2