Skip to content

Conversation

antoniovazquezblanco
Copy link
Contributor

Checklist:

  • If you are new to Scapy: I have checked CONTRIBUTING.md (esp. section submitting-pull-requests)
  • I squashed commits belonging together
  • I added unit tests or explained why they are not relevant
  • I executed the regression tests (using cd test && ./run_tests or tox)
  • If the PR is still not finished, please create a Draft Pull Request

Based on https://gitlab.com/LarsVoelker/wireshark/-/commit/3c4a5165a69d371d2abae1f271fbd806f9a31819, I improved the naming of the fields for this particular command.

The protocol is getting a little bit more complex and it may be time for splitting it into layers if it keeps growing. Let me know what do you think about that and I may open a new PR with that changes if you think it may help.

Thanks!

Copy link

codecov bot commented Aug 15, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 80.98%. Comparing base (d6a25bb) to head (cd7085c).
⚠️ Report is 7 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #4818      +/-   ##
==========================================
- Coverage   80.98%   80.98%   -0.01%     
==========================================
  Files         365      365              
  Lines       89112    89114       +2     
==========================================
  Hits        72167    72167              
- Misses      16945    16947       +2     
Files with missing lines Coverage Δ
scapy/contrib/automotive/bmw/hsfz.py 63.72% <100.00%> (+0.72%) ⬆️

... and 6 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@gpotter2 gpotter2 requested a review from Copilot August 22, 2025 08:32
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR improves the field naming in the HSFZ (BMW's proprietary automotive protocol) packet structure to better reflect the semantics of different control frames. Based on a Wireshark commit, the change clarifies that control type 0x40 (incorrect_tester_address) frames contain "expected" and "received" address fields rather than generic "source" and "target" fields.

  • Renamed generic address fields to more descriptive names for control type 0x40 frames
  • Split address field logic into separate methods for different control types
  • Updated test assertions to match the new field names

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
scapy/contrib/automotive/bmw/hsfz.py Added new conditional fields and split address detection logic for better semantic accuracy
test/contrib/automotive/bmw/hsfz.uts Updated test assertions to use new field names for control type 0x40 frames

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@polybassa
Copy link
Contributor

Thanks for the PR. For now, I would not split the layers.

@polybassa polybassa enabled auto-merge (squash) August 25, 2025 18:08
@polybassa polybassa merged commit 29433fc into secdev:master Aug 25, 2025
61 of 64 checks passed
@antoniovazquezblanco antoniovazquezblanco deleted the hsfz_test branch August 26, 2025 11:41
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