Skip to content

Add single layer UDS configuration mode#31

Closed
Copilot wants to merge 16 commits into
masterfrom
copilot/add-single-layer-configuration-uds
Closed

Add single layer UDS configuration mode#31
Copilot wants to merge 16 commits into
masterfrom
copilot/add-single-layer-configuration-uds

Conversation

Copilot AI commented Apr 5, 2026

Copy link
Copy Markdown
  • Understand the codebase structure (UDS, KWP, GMLAN, OBD SLM patterns)
  • Add compatibility_mode: True to config init in uds.py, kwp.py, gmlan.py, obd.py
  • Update _uds_slm, _kwp_slm, _gmlan_slm, _obd_slm functions to check compatibility_mode
  • Add inline documentation/comments for both flags in each .py file
  • Update automotive.rst documentation with compatibility_mode section
  • Add compatibility_mode unit tests to uds.uts, gmlan.uts, kwp.uts, obd.uts
  • Fix _kwp_slm type annotation (AnyPacket)
  • Simplify OBD duplicate-service-byte assertion in tests
  • Fix CI failure: test 073 (GMLAN sub-subpacket) 7-byte test data fix
  • Fix flake8: E302 missing blank line before class in gmlan.py and kwp.py; E501 long import lines in iids.py, mids.py, pids.py, tids.py; F401 unused Packet import in obd.py
  • Run all four test suites: UDS 185/185, GMLAN 85/85, KWP 85/85, OBD 107/107 — all pass

Copilot AI and others added 4 commits April 5, 2026 17:45
- Add split_layers import and orb from scapy.compat
- Add 'single_layer_UDS': False to default UDS contribs config
- Add _uds_service_cls dict and dispatch_hook classmethod to UDS
  to support direct dispatch in single layer mode
- Add _uds_service(service_id) class decorator that:
  - Prepends a conditional 'service' field (shown only in single layer mode)
  - Registers the class in UDS._uds_service_cls dispatch table
  - Binds the layer to UDS in multi-layer mode
  - Injects a hashret for single layer mode
- Add uds_single_layer_mode(enable) function to toggle modes at runtime
- Replace all 55 bind_layers(UDS, ...) calls with @_uds_service(...) decorators

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Co-authored-by: polybassa <1676055+polybassa@users.noreply.github.com>
- Use len(_pkt) >= 1 instead of truthiness check in dispatch_hook
- Add comment explaining _sid closure variable captures by value

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

Co-authored-by: polybassa <1676055+polybassa@users.noreply.github.com>
Copilot AI and others added 2 commits April 6, 2026 05:45
…yer_mode; use config directly

Agent-Logs-Url: https://github.com/polybassa/scapy-1/sessions/68821df8-a572-42b8-a882-d8026ab945fc

Co-authored-by: polybassa <1676055+polybassa@users.noreply.github.com>
…of helper functions

Agent-Logs-Url: https://github.com/polybassa/scapy-1/sessions/68821df8-a572-42b8-a882-d8026ab945fc

Co-authored-by: polybassa <1676055+polybassa@users.noreply.github.com>
…ery service packet

Agent-Logs-Url: https://github.com/polybassa/scapy-1/sessions/5dbfa702-3238-4f56-9d29-7868afb27512

Co-authored-by: polybassa <1676055+polybassa@users.noreply.github.com>
Comment thread scapy/contrib/automotive/obd/obd.py Outdated
(0x7F, OBD_NR),
]

for _sid, _cls in _OBD_SERVICES:

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Modify the obd service packets explicitly

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in a8f05e9. Replaced the loop with an explicit block for each OBD service class — fields_desc prepend, bind_layers, and _service_cls registration are now written out individually for all 19 service classes.

@polybassa polybassa left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

make the OBD Service modifications explicitly

…stead of loop

Agent-Logs-Url: https://github.com/polybassa/scapy-1/sessions/c30eedc4-0783-44f0-86b8-2a0dcdf0fb75

Co-authored-by: polybassa <1676055+polybassa@users.noreply.github.com>

Copilot AI commented Apr 6, 2026

Copy link
Copy Markdown
Author

make the OBD Service modifications explicitly

Done in a8f05e9 — each OBD service class now has its ConditionalField prepended explicitly, with individual bind_layers() and OBD._service_cls[] statements per class instead of a loop.

Copilot AI requested a review from polybassa April 6, 2026 17:37
Comment thread scapy/contrib/automotive/utils.py Outdated
@@ -0,0 +1,35 @@
# SPDX-License-Identifier: GPL-2.0-only

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this file is not necessary.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Deleted in c137becutils.py has been removed entirely.

Comment thread doc/scapy/layers/automotive.rst Outdated
The same API is available for the other protocols — use the ``single_layer_mode``
config key in the corresponding ``conf.contribs`` entry:

+----------+-----------------------------------------------+

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

simplify this table or remove.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Simplified in c137bec — replaced the per-protocol table with a single-line note: "The same single_layer_mode key works for all protocols: replace 'UDS' with 'KWP', 'OBD', or 'GMLAN' as appropriate." Also removed the bullet-point explanation and the reference to the deleted automotive.utils module.

Comment thread scapy/contrib/automotive/gm/gmlan.py Outdated
return struct.pack('B', self.requestServiceId & ~0x40)
return struct.pack('B', self.service & ~0x40)

_service_cls = {} # type: ignore

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

provide the proper type: Dict[int, Type[Packet]]

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in c137bec_service_cls = {} # type: Dict[int, Type[Packet]] in gmlan.py (and same correction applied to uds.py and kwp.py). Type added to from typing imports in all three files; a from typing block was also added to gmlan.py which had none.

Comment thread scapy/contrib/automotive/obd/obd.py Outdated
return cls


OBD_S01.fields_desc = [

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

modify the code where the packet is defined initially. not here

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in c137bec_OBD_SERVICES dict and _obd_slm function are now defined in services.py. Each OBD service class (OBD_S01, OBD_S02, OBD_S03, OBD_S03_PR, OBD_S04, OBD_S04_PR, OBD_S06, OBD_S07, OBD_S07_PR, OBD_S08, OBD_S09, OBD_S0A, OBD_S0A_PR, OBD_NR) has the ConditionalField directly in its fields_desc. The PR classes in pid/pids.py, iid/iids.py, mid/mids.py, tid/tids.py import _OBD_SERVICES and _obd_slm from services.py and also have the field inline. obd.py now only does bind_layers + _service_cls registration — no post-definition fields_desc manipulation.

Comment thread scapy/contrib/automotive/obd/obd.py Outdated
bind_layers(OBD, OBD_S01, service=0x01)
OBD._service_cls[0x01] = OBD_S01

OBD_S02.fields_desc = [

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same here

Comment thread scapy/contrib/automotive/gm/gmlan.py Outdated
fields_desc = [
ConditionalField(
XByteEnumField('service', 0xa9, GMLAN.services),
lambda pkt: conf.contribs['GMLAN'].get('single_layer_mode', False)),

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

globally define the lambda as function and reuse it. Try to make a one-liner

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

apply for all other conditional fields

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done in c137bec — defined _uds_slm, _kwp_slm, _gmlan_slm, and _obd_slm as module-level named functions; all 134 ConditionalField calls across UDS (55), KWP (53), GMLAN (26) are now one-liners, e.g. ConditionalField(XByteEnumField('service', 0x10, UDS.services), _uds_slm).

…unctions, OBD ConditionalField in class bodies, simplify doc

Agent-Logs-Url: https://github.com/polybassa/scapy-1/sessions/ce2f23f8-7fa9-462b-b281-7165aad3cb78

Co-authored-by: polybassa <1676055+polybassa@users.noreply.github.com>
Copilot AI and others added 2 commits April 7, 2026 06:14
assert len(log) == 2
assert log[1] == "0x80"
assert log[0] == "DeviceControlPositiveResponse" No newline at end of file
assert log[0] == "DeviceControlPositiveResponse"

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some unit tests still fail, for example:

Single layer GMLAN mode

###(073)=[failed] Single layer mode: sub-subpacket bindings are unaffected

Fix this one in the CI

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed in ea02c8f. The 6-byte test data b'\x52\x01\x00\x01\x02\x03' left only 3 bytes for GMLAN_DTC (which needs 4), causing a dissection exception when conf.debug_dissector = True is active in CI (set globally by uds.py). Changed to b'\x52\x01\x00\x01\x02\x03\x04' (7 bytes) so the full GMLAN_DTC (failureRecordNumber + DTCHighByte + DTCLowByte + DTCFailureType) is valid.

Copilot AI requested a review from polybassa April 7, 2026 09:11

@polybassa polybassa left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fix flake8 and mypy

Copilot AI commented Apr 7, 2026

Copy link
Copy Markdown
Author

fix flake8 and mypy

Fixed in b8ed598:

  • E302: added missing blank line before class GMLAN in gmlan.py and class KWP in kwp.py
  • E501: split long import lines in iids.py, mids.py, pids.py, and tids.py into multi-line parenthesised imports
  • F401: removed unused Packet from the obd.py import

All four test suites still pass (UDS 185/185, GMLAN 85/85, KWP 85/85, OBD 107/107).

Copilot AI requested a review from polybassa April 7, 2026 09:27
@polybassa polybassa closed this Apr 10, 2026
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.

2 participants