#1754 Add Modbus Support

[yahyayozo]

• Add ModbusLayer first implementation with header parsing only (no test included)

[codecov[bot]]

Codecov Report

:x: Patch coverage is 91.86992% with 10 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 83.47%. Comparing base (699f7b0) to head (e3461d0). :warning: Report is 6 commits behind head on dev.

Files with missing lines	Patch %	Lines
Packet++/src/ModbusLayer.cpp	88.05%	8 Missing :warning:
Packet++/header/ModbusLayer.h	90.00%	2 Missing :warning:

Additional details and impacted files

@@            Coverage Diff             @@
##              dev    #1823      +/-   ##
==========================================
- Coverage   83.47%   83.47%   -0.01%     
==========================================
  Files         298      304       +6     
  Lines       53945    54152     +207     
  Branches    11705    12013     +308     
==========================================
+ Hits        45029    45201     +172     
- Misses       7709     7735      +26     
- Partials     1207     1216       +9     

Flag	Coverage Δ
alpine320	75.45% <93.33%> (+0.02%)	:arrow_up:
fedora42	75.60% <93.44%> (+0.06%)	:arrow_up:
macos-13	81.67% <89.10%> (+<0.01%)	:arrow_up:
macos-14	81.67% <89.10%> (+<0.01%)	:arrow_up:
macos-15	81.67% <89.10%> (+<0.01%)	:arrow_up:
mingw32	70.29% <78.72%> (-0.01%)	:arrow_down:
mingw64	70.27% <78.72%> (-0.02%)	:arrow_down:
rhel94	75.30% <93.44%> (+0.02%)	:arrow_up:
ubuntu2004	59.24% <76.56%> (+0.03%)	:arrow_up:
ubuntu2004-zstd	59.36% <76.56%> (+0.05%)	:arrow_up:
ubuntu2204	75.23% <93.44%> (-0.01%)	:arrow_down:
ubuntu2204-icpx	60.93% <70.58%> (+0.04%)	:arrow_up:
ubuntu2404	75.50% <93.44%> (+0.03%)	:arrow_up:
ubuntu2404-arm64	75.47% <93.44%> (+0.02%)	:arrow_up:
unittest	83.47% <91.86%> (-0.01%)	:arrow_down:
windows-2022	85.49% <86.95%> (+0.01%)	:arrow_up:
windows-2025	85.51% <86.95%> (+0.01%)	:arrow_up:
winpcap	85.51% <86.95%> (+0.01%)	:arrow_up:
xdp	53.09% <93.44%> (+0.12%)	:arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:

• :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[yahyayozo]

@seladb I've prepared only 1 constructor which initiates the layer with default fields values Can you please mention what are the other constructors I need to implement? also please mention if I need to follow on other protocols layers?

[seladb]

@yahyayozo we usually have at least 2 c'tors, sometimes more:

1. As part of the packet parsing flow. For example: https://github.com/seladb/PcapPlusPlus/blob/a49a79e0b67b402ad75ffa96c1795def36df75c8/Packet%2B%2B/header/MplsLayer.h#L37 which is used here (and in some other places also): https://github.com/seladb/PcapPlusPlus/blob/a49a79e0b67b402ad75ffa96c1795def36df75c8/Packet%2B%2B/src/VlanLayer.cpp#L101
2. When the user wants to create a new layer of this type. This is the c'tor you already implemented, but usually we don't implement (only) c'tors with zero arguments. We tend to implement c'tors that allow the user to build the layer. In your case, I'd add arguments like transactionId, unitId and functionCode

[yahyayozo]

Hi @seladb can you please guide me briefly on how to add tests for my ModbusLayer for parsing/crafting headers? As I want to pass to function types request/response after testing the header part.

[seladb]

@yahyayozo sure! You can learn about tests here: https://pcapplusplus.github.io/docs/tests

At a high level:

• You should add your tests under Tests/Packet++Test/Tests (don't forget to add your .cpp file to CMakeFiles.txt)
• The tests use a homegrown testing framework. A test case is PTF_TEST_CASE. You can write 3 of those: for parsing, crafting, and editing
• You can find the possible assertions here, the most common one is PTF_ASSERT_EQUAL(actual, expected)
• You need to add your test cases to https://github.com/seladb/PcapPlusPlus/blob/master/Tests/Packet%2B%2BTest/main.cpp and add a PTF_RUN_TEST for each test case
• When you build the tests, you should run them from the Tests/Packet++Test directory, like this: Bin/Packet++Test -t <YOUR_TEST_CASE>

Please look at similar tests that were written for other layers and follow the same patterns.

Please let me know if you have any questions.

[yahyayozo]

@seladb I'll add tests for the headers crafting/parsing implementation and then you can a review before we move to adding the different PDU types

[yahyayozo]

@seladb @Dimi1010 I've started adding some tests, but I have a question regarding the ".dat" files; how can I generate examples for some Modbus frames?

[seladb]

@seladb @Dimi1010 I've started adding some tests, but I have a question regarding the ".dat" files; how can I generate examples for some Modbus frames?

If you open the file in Wireshark, you can choose a packet and right click -> Copy -> ... as a Hex Stream:

[seladb]

@yahyayozo CI fails, can you please look into it? 🙁

[yahyayozo]

@yahyayozo CI fails, can you please look into it? 🙁

[yahyayozo]

@seladb I'll check further

[yahyayozo]

@seladb all the pre-commit checks have passed + the build was successful I think if the pipeline build fails, I need to check my compiler, maybe I'm using a wrong version

[yahyayozo]

@seladb CI passed! can you please make a review of the already done work before moving forward?

[yahyayozo]

@seladb Hi, can you please check the progress I've reached until now? please leave comments on any part that needs adjustement

[seladb]

@seladb Hi, can you please check the progress I've reached until now? please leave comments on any part that needs adjustement

@yahyayozo sure, I'm sorry for the long delay. I'll take a look shortly

[yahyayozo]

@seladb pcap files are ignored, in the gitignore, how can I commit my modbus.pcap file?

[seladb]

@seladb pcap files are ignored, in the gitignore, how can I commit my modbus.pcap file?

@yahyayozo you can use git add -f modbus.pcap to force adding a pcap file despite of .gitignore

[yahyayozo]

@seladb I've made the changes of your last review, and the pipeline is working I've also included the pcap file for Modbus Can you please review again?

[seladb]

@yahyayozo please scroll up and see all the comments that weren't addressed

[seladb]

@yahyayozo the tests fail in CI...

[yahyayozo]

@seladb should be fixed now

[seladb]

@yahyayozo please also see older comments:

• https://github.com/seladb/PcapPlusPlus/pull/1823#discussion_r2265164208
• https://github.com/seladb/PcapPlusPlus/pull/1823#discussion_r2278509145
• https://github.com/seladb/PcapPlusPlus/pull/1823#discussion_r2287041041
• https://github.com/seladb/PcapPlusPlus/pull/1823#discussion_r2287041757
• https://github.com/seladb/PcapPlusPlus/pull/1823#discussion_r2278514017

[seladb]

@yahyayozo I think the only comments left are:

• https://github.com/seladb/PcapPlusPlus/pull/1823#discussion_r2278509145
• https://github.com/seladb/PcapPlusPlus/pull/1823#discussion_r2278514017

Once you address those I think we can merge this PR

[yahyayozo]

@seladb all the comments shall be addressed now I kept the ModbusReadInputRegisters struct because I use in the constructor and also to keep it as an example for the future implementation of other functions Codes

[yahyayozo]

@seladb can you please check why it shows forbid submodules in the failed pre-commit?

[Dimi1010]

@seladb can you please check why it shows forbid submodules in the failed pre-commit?

a7b926daa892f6d5242076be0e1f63c8301cdf95 this commit adds deps/benchmark-src to the repo. I think this is a mistake.

[yahyayozo]

@seladb @Dimi1010 I removed it I don't know how it was added as a submodule!!

[seladb]

@yahyayozo thank you so much for working on this and contributing to PcapPlusPlus, it is much appreciated! 🙏

[yahyayozo]

@seladb so, the other PDUs types will added in another PR?

[Dimi1010]

Yes, it's easier to review that way as it results in smaller differential changes in PRs.