#1754 Add Modbus Support #1823

yahyayozo · 2025-05-18T18:15:25Z

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

codecov · 2025-05-18T19:05:44Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.88%. Comparing base (92c6b5a) to head (b96671c).

Additional details and impacted files

@@            Coverage Diff             @@
##              dev    #1823      +/-   ##
==========================================
+ Coverage   82.84%   82.88%   +0.03%     
==========================================
  Files         291      294       +3     
  Lines       51553    51663     +110     
  Branches    11395    11172     -223     
==========================================
+ Hits        42709    42820     +111     
+ Misses       8022     8020       -2     
- Partials      822      823       +1

Flag	Coverage Δ
alpine320	`74.63% <92.98%> (+0.03%)`	⬆️
fedora42	`74.76% <93.10%> (+0.04%)`	⬆️
macos-13	`81.10% <98.86%> (+0.03%)`	⬆️
macos-14	`81.10% <98.86%> (+0.03%)`	⬆️
macos-15	`81.10% <98.86%> (+0.03%)`	⬆️
mingw32	`69.87% <70.21%> (-0.04%)`	⬇️
mingw64	`69.86% <70.21%> (-0.04%)`	⬇️
npcap	`84.83% <100.00%> (+0.02%)`	⬆️
rhel94	`74.49% <93.10%> (+0.03%)`	⬆️
ubuntu2004	`58.81% <79.03%> (+0.03%)`	⬆️
ubuntu2004-zstd	`58.92% <79.03%> (+0.05%)`	⬆️
ubuntu2204	`74.43% <92.98%> (+0.05%)`	⬆️
ubuntu2204-icpx	`60.88% <83.92%> (+0.10%)`	⬆️
ubuntu2404	`74.64% <92.98%> (+0.01%)`	⬆️
ubuntu2404-arm64	`74.64% <92.98%> (+0.05%)`	⬆️
unittest	`82.88% <100.00%> (+0.03%)`	⬆️
windows-2022	`84.74% <100.00%> (-0.04%)`	⬇️
windows-2025	`84.87% <100.00%> (+0.03%)`	⬆️
winpcap	`84.98% <100.00%> (+0.04%)`	⬆️
xdp	`51.37% <92.98%> (+0.07%)`	⬆️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

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

yahyayozo · 2025-05-21T20:14:44Z

@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 · 2025-05-22T06:53:53Z

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

As part of the packet parsing flow. For example:

PcapPlusPlus/Packet++/header/MplsLayer.h

Line 37 in a49a79e

MplsLayer(uint8_t* data, size_t dataLen, Layer* prevLayer, Packet* packet)

which is used here (and in some other places also):

PcapPlusPlus/Packet++/src/VlanLayer.cpp

Line 101 in a49a79e

m_NextLayer = new MplsLayer(payload, payloadLen, this, m_Packet);
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

…sPlus into feature/modbus

yahyayozo · 2025-05-24T10:48:32Z

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 · 2025-05-25T07:11:43Z

@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 · 2025-05-28T21:59:03Z

@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 · 2025-06-02T20:08:24Z

@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 · 2025-06-02T21:58:05Z

@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:

… instead of raw data

seladb · 2025-06-12T03:18:28Z

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

yahyayozo · 2025-06-12T18:35:24Z

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

yahyayozo · 2025-06-12T18:36:30Z

@seladb I'll check further

…ion.h

yahyayozo · 2025-06-15T19:44:08Z

@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

… transaction ID, unit ID, and function code

…sPlus into feature/modbus

yahyayozo · 2025-06-20T21:11:41Z

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

yahyayozo · 2025-07-19T20:30:45Z

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

seladb · 2025-07-19T20:51:23Z

@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

seladb · 2025-07-20T02:44:44Z

Tests/Packet++Test/PacketExamples/ModbusRequest.dat

Can you add the pcap/pcapng file you used also?
It'll also help me understand the protocol better

seladb · 2025-07-20T02:45:15Z

Packet++/header/ProtocolType.h

Can you also add this protoclo to README.md?

seladb · 2025-07-20T02:48:16Z

Packet++/header/ModbusLayer.h

+		}
+
+		/// @return A pointer to the MODBUS header
+		modbus_common_header* getModbusHeader() const;


This could be a private method?

seladb · 2025-07-20T02:52:08Z

Packet++/header/ModbusLayer.h

+#pragma pack(push, 1)
+	/// @struct modbus_common_header
+	/// MODBUS Application Protocol header
+	struct modbus_common_header


nit: I think we can call it just modbus_header?

seladb · 2025-07-20T02:53:19Z

Packet++/header/ModbusLayer.h

+
+		/// @brief  Check if a port is a valid MODBUS port
+		/// @param port
+		/// @return


nit: complete the return documentation?

seladb · 2025-07-20T02:55:43Z

Packet++/header/ModbusLayer.h

+		std::string toString() const override
+		{
+			return "Modbus Layer, Transaction ID: " + std::to_string(getTransactionId()) +
+			       ", Protocol ID: " + std::to_string(getProtocolId()) + ", Length: " + std::to_string(getLength()) +
+			       ", Unit ID: " + std::to_string(getUnitId()) +
+			       ", Function Code: " + std::to_string(getFunctionCode());
+		}


Maybe move the implementation to the .cpp file to keep the .h file cleaner?

I'm not sure how Modbus packets look in Wireshark, but probably contain less info? 🤔

seladb · 2025-07-20T06:58:15Z

Tests/Packet++Test/Tests/ModbusTests.cpp

+
+	pcpp::ModbusLayer* modbusLayer = packet.getLayerOfType<pcpp::ModbusLayer>();
+	PTF_ASSERT_NOT_NULL(modbusLayer);
+	PTF_ASSERT_EQUAL(modbusLayer->getTransactionId(), 0);


Can you get a packet where transaction ID is not 0?

seladb · 2025-07-20T06:59:35Z

Tests/Packet++Test/Tests/ModbusTests.cpp

+	PTF_ASSERT_EQUAL(modbusLayer.getFunctionCode(), 3);
+	PTF_ASSERT_EQUAL(modbusLayer.getHeaderLen(), sizeof(pcpp::modbus_common_header));
+
+	PTF_ASSERT_EQUAL(modbusLayer.toString(),


I'd test toString() in ModbusLayerParsingTest instead of here

seladb · 2025-07-20T07:02:44Z

Tests/Packet++Test/Tests/ModbusTests.cpp

+#include "EndianPortable.h"
+#include "SystemUtils.h"
+
+PTF_TEST_CASE(ModbusLayerCreationTest)


The best way to test layer creation is to compare it to a real packet. You can load ModbusRequest.dat, extract the modbus layer and compare it to the created packet in this test

yahyayozo added 2 commits May 18, 2025 19:09

Add ModbusLayer first implementation with header parsing only

a36e594

fix static_assert

88f69d4

yahyayozo requested a review from seladb as a code owner May 18, 2025 18:15

Merge branch 'dev' into feature/modbus

c257494

yahyayozo marked this pull request as draft May 18, 2025 18:16

Dimi1010 linked an issue May 19, 2025 that may be closed by this pull request

Add Modbus Protocol Support #1813

Open

Dimi1010 added the enhancement label May 20, 2025

Fix formatting in ModbusLayer header file

348afc6

yahyayozo marked this pull request as ready for review May 21, 2025 20:13

yahyayozo added 2 commits May 21, 2025 21:45

Add setter methods for MODBUS header fields in ModbusLayer

1f65c24

Merge branch 'dev' into feature/modbus

4159c6c

yahyayozo added 6 commits May 23, 2025 19:02

Refactor ModbusLayer class documentation and fix syntax error

58c18ec

Merge branch 'feature/modbus' of https://github.com/yahyayozo/PcapPlu…

d50db3b

…sPlus into feature/modbus

Fix endianess handling in ModbusLayer getters and setters

cfcbc69

Add const qualifier to ModbusLayer getter methods

28dd176

Add constructors to ModbusLayer for initializing from user inputs

34d65a2

Add Modbus protocol constant to ProtocolType

74ab79d

Add const qualifier to ModbusLayer getter methods in .h file

166c15c

yahyayozo marked this pull request as draft May 28, 2025 22:00

yahyayozo added 2 commits May 29, 2025 22:20

Add ModbusLayer and corresponding tests to Packet++Test

3624286

Fix length assignment in ModbusLayer constructor to use htobe16

bcd14b5

Refactor ModbusLayerParsingTest to create ModbusLayer from a dat file…

9f4ebbd

… instead of raw data

yahyayozo closed this Jun 12, 2025

yahyayozo reopened this Jun 13, 2025

yahyayozo added 4 commits June 15, 2025 19:05

Fix formatting in ModbusLayer constructor for improved readability

4ceb41d

Remove redundant ModbusLayerParsingTest case from ModbusTests.cpp

adb3f51

Remove ModbusLayerParsingTest from test definitions and main execution

839aa49

Fix newline at end of file in ModbusLayerCreationTest and TestDefinit…

dc04c0c

…ion.h

yahyayozo added 5 commits June 16, 2025 21:16

Add ModbusLayer parsing test and utility function for port validation

f4f5291

Merge branch 'dev' into feature/modbus

24c0924

Add missing includes for iostream and iomanip in ModbusLayer.cpp

32da5e5

Refactor ModbusLayerCreationTest to include additional assertions for…

da52bf7

… transaction ID, unit ID, and function code

Merge branch 'feature/modbus' of https://github.com/yahyayozo/PcapPlu…

c8bd099

…sPlus into feature/modbus

seladb reviewed Jul 20, 2025

View reviewed changes

Merge branch 'dev' into feature/modbus

b96671c

#1754 Add Modbus Support #1823

Are you sure you want to change the base?

#1754 Add Modbus Support #1823

Uh oh!

Conversation

yahyayozo commented May 18, 2025

Uh oh!

codecov bot commented May 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

yahyayozo commented May 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

seladb commented May 22, 2025

Uh oh!

yahyayozo commented May 24, 2025

Uh oh!

seladb commented May 25, 2025

Uh oh!

yahyayozo commented May 28, 2025

Uh oh!

yahyayozo commented Jun 2, 2025

Uh oh!

seladb commented Jun 2, 2025

Uh oh!

seladb commented Jun 12, 2025

Uh oh!

yahyayozo commented Jun 12, 2025

Uh oh!

yahyayozo commented Jun 12, 2025

Uh oh!

yahyayozo commented Jun 15, 2025

Uh oh!

yahyayozo commented Jun 20, 2025

Uh oh!

yahyayozo commented Jul 19, 2025

Uh oh!

seladb commented Jul 19, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov bot commented May 18, 2025 •

edited

Loading

yahyayozo commented May 21, 2025 •

edited

Loading