You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@plc4x.apache.org by "jsxs0 (via GitHub)" <gi...@apache.org> on 2025/02/15 14:50:11 UTC
[PR] feat(rust): Add initial S7 protocol implementation
jsxs0 opened a new pull request, #2010:
URL: https://github.com/apache/plc4x/pull/2010
## Overview
Initial implementation of the S7 protocol in Rust for Apache PLC4X, focusing on memory safety and performance.
### Features
- Memory-safe, zero-copy protocol parsing using nom
- Async/await support with Tokio runtime
- Comprehensive test coverage including fuzz testing
- Industrial-grade error handling
### Implementation Details
- S7 protocol header parsing and validation
- Async connection negotiation
- Type-safe protocol enums (AreaCode, FunctionCode, etc.)
- Structured error handling with thiserror
### Testing
The implementation includes:
- Unit tests for protocol parsing
- Async connection tests
- Fuzz testing for header parsing
- CI integration with cross-platform support
## Testing Instructions
```bash
# Run unit tests
cargo test
# Run fuzzer
cargo install cargo-fuzz
cargo fuzz run header_parser
```
## Checklist
- [x] Documentation added
- [x] Tests implemented
- [x] CI configuration added
- [x] Error handling implemented
- [x] Async support added
- [x] Fuzz testing configured
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@plc4x.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(rust): Add initial S7 protocol implementation
Posted by "chrisdutz (via GitHub)" <gi...@apache.org>.
chrisdutz commented on PR #2010:
URL: https://github.com/apache/plc4x/pull/2010#issuecomment-2668106207
I guess in order to review the actual code, it would probably help, if I understood Rust ;-)
I can give you some pointers to what's generally important when bringing plc4x to new languages:
- Build an SPI, so a lot of the general purpose code can be shared among protocol-drivers
- Support the concept of transports: serial, udp, tcp, (raw-socket) ... and "test"
The following would be for when the code-gen has generally been implemented:
- Implement a runner to run the xml-based ParserSerializer tests (That are deployed along the mspec-protocol modules) against your generated code
- Implement a runner to run the xml-based Integration tests (That are deployed along the mspec-protocol modules) against your driver
I'll try to have a look and play around with your driver. Thanks for contributing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@plc4x.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(rust): Add initial S7 protocol implementation
Posted by "chrisdutz (via GitHub)" <gi...@apache.org>.
chrisdutz commented on PR #2010:
URL: https://github.com/apache/plc4x/pull/2010#issuecomment-2668116074
So from a quick look, I guess we'll need a Maven integration to allow a normal maven build to trigger the rust build, but I think we can borrow things from Julian's branch. Possibly I could create PRs for your PR ... Let me see which form of collaboration works best.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@plc4x.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(plc4rs): Add transport layer with TCP/UDP implementations
Posted by "chrisdutz (via GitHub)" <gi...@apache.org>.
chrisdutz commented on PR #2010:
URL: https://github.com/apache/plc4x/pull/2010#issuecomment-2698598142
With those two changes, my build is currently producing this output:
```
[INFO] --------------------< org.apache.plc4x:plc4x-rust >---------------------
[INFO] Building PLC4Rust 0.13.0-SNAPSHOT
[INFO] from pom.xml
[INFO] --------------------------------[ pom ]---------------------------------
[INFO]
[INFO] --- clean:3.4.0:clean (default-clean) @ plc4x-rust ---
[INFO] Deleting /Users/christoferdutz/Projects/Apache/PLC4X/plc4x/plc4x-rust/target
[INFO]
[INFO] --- enforcer:3.5.0:enforce (enforce-maven-version) @ plc4x-rust ---
[INFO] Rule 0: org.apache.maven.enforcer.rules.version.RequireMavenVersion passed
[INFO]
[INFO] --- enforcer:3.5.0:enforce (enforce-java-version) @ plc4x-rust ---
[INFO] Rule 0: org.apache.maven.enforcer.rules.version.RequireJavaVersion passed
[INFO]
[INFO] --- enforcer:3.5.0:enforce (enforce-minimum-maven-version) @ plc4x-rust ---
[INFO] Rule 0: org.apache.maven.enforcer.rules.version.RequireMavenVersion passed
[INFO]
[INFO] --- enforcer:3.5.0:enforce (enforce-java-compatability) @ plc4x-rust ---
[INFO] Rule 0: org.codehaus.mojo.extraenforcer.dependencies.EnforceBytecodeVersion passed
[INFO]
[INFO] --- enforcer:3.5.0:enforce (enforce-version-convergence) @ plc4x-rust ---
[INFO] Rule 0: org.apache.maven.enforcer.rules.dependency.DependencyConvergence passed
[INFO]
[INFO] --- enforcer:3.5.0:enforce (enforce-ban-duplicate-classes) @ plc4x-rust ---
[INFO] Rule 0: org.codehaus.mojo.extraenforcer.dependencies.BanDuplicateClasses passed
[INFO]
[INFO] --- remote-resources:3.3.0:process (process-resource-bundles) @ plc4x-rust ---
[INFO] Preparing remote bundle org.apache.apache.resources:apache-jar-resource-bundle:1.7
[INFO] Copying 3 resources from 1 bundle.
[INFO]
[INFO] --- rust:1.2.0:build (rust-build-id) @ plc4x-rust ---
[INFO] Working directory: /Users/christoferdutz/Projects/Apache/PLC4X/plc4x/plc4x-rust
[INFO] Running: cargo build --target-dir /Users/christoferdutz/Projects/Apache/PLC4X/plc4x/plc4x-rust/target/rust-maven-plugin/plc4x-rust
[INFO] Compiling proc-macro2 v1.0.93
[INFO] Compiling unicode-ident v1.0.16
[INFO] Compiling libc v0.2.169
[INFO] Compiling autocfg v1.4.0
[INFO] Compiling parking_lot_core v0.9.10
[INFO] Compiling smallvec v1.14.0
[INFO] Compiling scopeguard v1.2.0
[INFO] Compiling cfg-if v1.0.0
[INFO] Compiling once_cell v1.20.3
[INFO] Compiling pin-project-lite v0.2.16
[INFO] Compiling thiserror v1.0.69
[INFO] Compiling minimal-lexical v0.2.1
[INFO] Compiling bytes v1.10.0
[INFO] Compiling memchr v2.7.4
[INFO] Compiling tracing-core v0.1.33
[INFO] Compiling lock_api v0.4.12
[INFO] Compiling nom v7.1.3
[INFO] Compiling quote v1.0.38
[INFO] Compiling syn v2.0.98
[INFO] Compiling signal-hook-registry v1.4.2
[INFO] Compiling socket2 v0.5.8
[INFO] Compiling mio v1.0.3
[INFO] Compiling parking_lot v0.12.3
[INFO] Compiling thiserror-impl v1.0.69
[INFO] Compiling tracing-attributes v0.1.28
[INFO] Compiling tokio-macros v2.5.0
[INFO] Compiling tokio v1.43.0
[INFO] Compiling tracing v0.1.41
[INFO] Compiling plc4x-rust v0.1.0 (/Users/christoferdutz/Projects/Apache/PLC4X/plc4x/plc4x-rust)
[INFO] warning: unused import: `bytes::BytesMut`
[INFO] --> src/s7.rs:6:5
[INFO] |
[INFO] 6 | use bytes::BytesMut;
[INFO] | ^^^^^^^^^^^^^^^
[INFO] |
[INFO] = note: `#[warn(unused_imports)]` on by default
[INFO]
[INFO] warning: fields `protocol_id`, `reserved`, `pdu_reference`, `parameter_length`, and `data_length` are never read
[INFO] --> src/s7.rs:19:5
[INFO] |
[INFO] 18 | pub struct S7Header {
[INFO] | -------- fields in this struct
[INFO] 19 | protocol_id: u8,
[INFO] | ^^^^^^^^^^^
[INFO] 20 | message_type: MessageType,
[INFO] 21 | reserved: u16,
[INFO] | ^^^^^^^^
[INFO] 22 | pdu_reference: u16,
[INFO] | ^^^^^^^^^^^^^
[INFO] 23 | parameter_length: u16,
[INFO] | ^^^^^^^^^^^^^^^^
[INFO] 24 | data_length: u16,
[INFO] | ^^^^^^^^^^^
[INFO] |
[INFO] = note: `S7Header` has derived impls for the traits `Clone` and `Debug`, but these are intentionally ignored during dead code analysis
[INFO] = note: `#[warn(dead_code)]` on by default
[INFO]
[INFO] warning: `plc4x-rust` (lib) generated 2 warnings (run `cargo fix --lib -p plc4x-rust` to apply 1 suggestion)
[INFO] Finished `dev` profile [unoptimized + debuginfo] target(s) in 7.31s
[INFO]
[INFO] --- apache-rat:0.16.1:check (license-check) @ plc4x-rust ---
[INFO] Rat check: Summary over all files. Unapproved: 9, unknown: 9, generated: 1, approved: 2 licenses.
[WARNING] Files with unapproved licenses:
Cargo.toml
CONTRIBUTING.md
README.md
fuzz/Cargo.toml
fuzz/fuzz_targets/header_parser.rs
src/types.rs
src/error.rs
src/lib.rs
src/s7.rs
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time: 8.343 s
[INFO] Finished at: 2025-03-04T19:47:18+01:00
[INFO] ------------------------------------------------------------------------
```
The failure is related to missing Apache headers in the files listed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@plc4x.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(rust): Add initial S7 protocol implementation
Posted by "chrisdutz (via GitHub)" <gi...@apache.org>.
chrisdutz commented on PR #2010:
URL: https://github.com/apache/plc4x/pull/2010#issuecomment-2660970768
Cool :-)
This way I might even have some chance to start learning rust ;-)
As long as possible, it would be cool, if you could structurally stick to the structures as we define them in the s7 mspec, that will ensure that we can convert your manually written code into generated one a lot easier: https://github.com/apache/plc4x/blob/develop/protocols/s7/src/main/resources/protocols/s7/s7.mspec
Also when looking at the java implementation, don't let yourself be confused ... there is also a subscription protocol implemented in parallel in there, which complicates things quite a bit. Usually a reason why I like to start off with Modbus, for porting PLC4X to new languages ... it's definitely the simplest protocol.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@plc4x.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(plc4rs): Add transport layer with TCP/UDP implementations
Posted by "chrisdutz (via GitHub)" <gi...@apache.org>.
chrisdutz commented on PR #2010:
URL: https://github.com/apache/plc4x/pull/2010#issuecomment-2698595908
Soooooo ... when checking out your PR, I see two directories "plc4rs" and "plc4x-rust" ... which is used for what?
However it looked as if plc4x-rust seems to the the interesting directory. I took the liberty to add a pom.xml to that, which runs the build within the maven build.
A few things, I would like to ask you to change:
- Call the directory "plc4rs" or "plc4rust" (without the x and without a dash) (I would probably choose the "plc4rs" to match the naming of the other modules.
- We need Apache headers on all source-files
Here's the pom file for calling the build from within maven:
```
<?xml version="1.0" encoding="UTF-8"?>
<!--
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to you under the Apache License, Version 2.0 (the
"License"); you may not use this file except in compliance
with the License. You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
-->
<project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
<modelVersion>4.0.0</modelVersion>
<parent>
<groupId>org.apache.plc4x</groupId>
<artifactId>plc4x-parent</artifactId>
<version>0.13.0-SNAPSHOT</version>
</parent>
<artifactId>plc4x-rust</artifactId>
<packaging>pom</packaging>
<name>PLC4Rust</name>
<description>Implementation of the protocol adapters for usage as Rust module.</description>
<build>
<plugins>
<!-- Build the project -->
<plugin>
<groupId>org.questdb</groupId>
<artifactId>rust-maven-plugin</artifactId>
<version>1.2.0</version>
<executions>
<execution>
<id>rust-build-id</id>
<goals>
<goal>build</goal>
</goals>
<configuration>
<path>${project.basedir}</path>
<!--copyTo>${project.build.directory}/classes/io/questdb/jni/example/rust/libs</copyTo-->
<copyWithPlatformDir>true</copyWithPlatformDir>
</configuration>
</execution>
</executions>
</plugin>
</plugins>
</build>
</project>
```
In order to have it executed by the main project, you would need to add this profile to the parent pom (Root of the plc4x project)
```
<profile>
<id>with-rust</id>
<modules>
<module>plc4x-rust</module>
</modules>
</profile>
```
With that you should then be able to call the build with:
mvn -Pwith-rust clean install
We would need to also add code to the prerequisite check in order to give people some hints at what's missing in order to build the project.
If you could give me access to your fork? I could probably work on this with you.
Another alternative would be I push your branch into the plc4x repo ... however then you would need to do PRs against that
Another alternative would be, I create a fork of your repo and then create PRs against that (Which feels a bit silly)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@plc4x.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(rust): Add initial S7 protocol implementation
Posted by "jsxs0 (via GitHub)" <gi...@apache.org>.
jsxs0 commented on PR #2010:
URL: https://github.com/apache/plc4x/pull/2010#issuecomment-2673161526
@chrisdutz No worries at all about the timing, Chris! Enjoy the music festival and your remaining sabbatical time.
While you're away, I'll focus on:
1. Implementing UDP transport following the same pattern as TCP
2. Starting on the Maven integration (using Julian's branch as reference)
3. Adding more test cases, especially for error scenarios
I'll keep the changes small and focused to make review easier when you have time. Looking forward to your deeper look whenever you're back!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@plc4x.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(rust): Add initial S7 protocol implementation
Posted by "jsxs0 (via GitHub)" <gi...@apache.org>.
jsxs0 commented on PR #2010:
URL: https://github.com/apache/plc4x/pull/2010#issuecomment-2670438968
@chrisdutz Hey Chris,
Added the initial SPI layer with a basic TCP transport implementation. Kept it focused for now:
1. Transport trait with connect/read/write/close
2. TCP implementation with retry support
3. Basic config system for timeouts and TCP options
4. Error handling and logging
Let me know if this looks OK as a starting point. Planning to add UDP and Serial support (in follow-up PRs).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@plc4x.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(plc4rs): Add transport layer with TCP/UDP implementations
Posted by "chrisdutz (via GitHub)" <gi...@apache.org>.
chrisdutz commented on PR #2010:
URL: https://github.com/apache/plc4x/pull/2010#issuecomment-2698606805
Also I should report, that the project builds fine in RustRover ;-)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@plc4x.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(plc4rs): Add transport layer with TCP/UDP implementations
Posted by "jsxs0 (via GitHub)" <gi...@apache.org>.
jsxs0 commented on PR #2010:
URL: https://github.com/apache/plc4x/pull/2010#issuecomment-2705614346
@chrisdutz Thanks for the feedback! I've made all the changes you requested:
1. Renamed the directory to `plc4rs` and removed the old `plc4x-rust` directory
2. Added Apache license headers to all files (Rust source files, Cargo.toml, etc.)
3. Set up the Maven integration with your pom.xml file
4. Added the `with-rust` profile to the parent pom.xml
The build now works with `mvn -Pwith-rust clean install` and passes all the license checks. There are still some Rust warnings about unused variables, but those don't affect the build.
I've also added a README with basic usage instructions and updated the project structure to match PLC4X conventions.
Let me know if there's anything else I should fix!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@plc4x.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(rust): Add initial S7 protocol implementation
Posted by "jsxs0 (via GitHub)" <gi...@apache.org>.
jsxs0 commented on PR #2010:
URL: https://github.com/apache/plc4x/pull/2010#issuecomment-2660968294
@chrisdutz Thank you for the warm welcome, Chris!
Yes, you understood correctly - I'm currently implementing the core types manually to better understand the protocol intricacies and establish a solid foundation for the Rust implementation. This approach helps ensure we leverage Rust's type system and safety features effectively while maintaining high performance.
Thank you for pointing out Julian's branch - I'll definitely take a look at it. While starting fresh helped me understand the requirements better, there might be valuable insights there that could inform this implementation.
I'm very interested in learning more about the code generation approach you mentioned. Once I have the core types and parsing logic stabilized, I'd greatly appreciate your guidance on integrating with PLC4X's code generation system.
Currently focusing on:
1. Zero-copy parsing with nom
2. Type-safe protocol representations
3. Async connection handling
I'm hoping to contribute this as part of GSoC 2025, so I wanted to get an early start to demonstrate commitment and gather community feedback. Would you prefer I continue with the manual implementation for now, or would you like to discuss code generation integration earlier in the process?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@plc4x.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(rust): Add initial S7 protocol implementation
Posted by "chrisdutz (via GitHub)" <gi...@apache.org>.
chrisdutz commented on PR #2010:
URL: https://github.com/apache/plc4x/pull/2010#issuecomment-2660966314
Oh wow :-)
Welcome to the project ... this is really something super interesting for us. I had it on my todo list for way too long.
Do I understand the PR correctly, that you are currently manually implementing the types needed in order to create the driver? If yes, that's absolutely fine and usually the way I started porting PLC4X to other languages. Once you have most parts implemented, I will definitely help you port that to have that code generated. Please ping me, if you need help with that.
I know we have a super-ancient branch in our repo, where Julian once started working on something like that ... perhaps that could also be helpful for you? (https://github.com/apache/plc4x/tree/feature/plc4rs/plc4rust)
Chris
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@plc4x.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(rust): Add initial S7 protocol implementation
Posted by "chrisdutz (via GitHub)" <gi...@apache.org>.
chrisdutz commented on PR #2010:
URL: https://github.com/apache/plc4x/pull/2010#issuecomment-2670854655
Wie... Great stuff... I'll look at it ASAP, however it might take a bit as I'm traveling to a music festival over the weekend and next week is the last of my 2,5 month sabbatical... Still some things to finish. Just want you to know that you're not frustrated why I'm so slow... It's not lack of interest, it's too much stuff going on.
Looking forward to getting the time to have a deeper lookm
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@plc4x.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(rust): Add initial S7 protocol implementation
Posted by "jsxs0 (via GitHub)" <gi...@apache.org>.
jsxs0 commented on PR #2010:
URL: https://github.com/apache/plc4x/pull/2010#issuecomment-2667337253
@chrisdutz I've made some changes:
1. Is the alignment with s7.mspec correct, I mean, for parameter types?
2. Are there any protocol features missing from this initial implementation?
3. Is the error handling approach sufficient for industrial use?
4. Should we add more specific test cases?
---
For now, I'm thinking of the following next steps:
1. Add async connection handling
2. Implement message serialization
3. Add more protocol-specific error cases
4. Expand test coverage
5. Add documentation
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@plc4x.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(rust): Add initial S7 protocol implementation
Posted by "jsxs0 (via GitHub)" <gi...@apache.org>.
jsxs0 commented on PR #2010:
URL: https://github.com/apache/plc4x/pull/2010#issuecomment-2660971803
@chrisdutz Thanks Chris! Happy to help you get started with Rust too.
The s7.mspec is really helpful - I'll make sure my Rust structs follow these definitions closely. Makes sense about keeping it compatible with the code generation system.
About the subscription protocol - I think I'll start with just the basic S7 protocol first to keep things clean. I can look at Modbus too if you think that would be more helpful for understanding the overall architecture.
Let me update the current implementation to match the mspec and I'll push the changes soon.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscribe@plc4x.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] feat(plc4rs): Add transport layer with TCP/UDP implementations
Posted by "chrisdutz (via GitHub)" <gi...@apache.org>.
chrisdutz commented on PR #2010:
URL: https://github.com/apache/plc4x/pull/2010#issuecomment-4919058342
Sorry for such a long silence from my side ... a lot happened since then on my side ...
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@plc4x.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org