Hi Guys,
I’m trying to add a new binding to openhab. It is derived from modbus binding. The only difference is that instead of TCP, this one is using UDP and all the protocol queries have a subnetId byte in addition to the deviceId and functionId.
For this, I opened the following two pull requests:
openhab:main
← cipianpascu:feature/s-bus
opened 09:44AM - 07 Jan 24 UTC
This is the first attempt to implement the S-Bus G4 transport protocol.
The cod… e is derived from the Modbus transport protocol and has the following two additions:
1. TCP protocol is replaced with UDP protocol
2. All the protocol queries have a subnetId byte in addition to the deviceId and functionId.
Please have a look to what I did, and let me know about the additional changes/fixes required for this new transport protocol/binding.
openhab:main
← cipianpascu:feature/s-bus
opened 09:49AM - 07 Jan 24 UTC
This is the first attempt to implement the S-Bus G4 transport protocol.
The cod… e is derived from the Modbus transport protocol and has the following two additions:
1. TCP protocol is replaced with UDP protocol
2. All the protocol queries have a subnetId byte in addition to the deviceId and functionId.
Please look at what I did, and let me know about the additional changes/fixes required for this new transport protocol/binding.
EDIT: Due to SBUS complexity, I diverged from the modbus protocol. This is the new PR:
openhab:main
← cipianpascu:feature/s-bus
opened 07:46AM - 02 Jan 25 UTC
<!--
Thanks for contributing to the openHAB project!
Please describe the goal … and effect of your PR here.
Pay attention to the below notes and to *the guidelines* for this repository.
Feel free to delete any comment sections in the template (starting with "<!--").
ATTENTION: Don't use "git merge" when working with your pull request branch!
This can clutter your Git history and make your PR unusable.
Use "git rebase" instead. See this forum post for further details:
https://community.openhab.org/t/rebase-your-code-or-how-to-fix-your-git-history-before-requesting-a-pull/129358
All PRs should be created using the "main" branch as base.
Important bugfixes are cherry-picked by maintainers to the patch release branch after a PR has been merged.
Add one or more appropriate labels to make your PR show up in the release notes.
E.g. enhancement, bug, documentation, new binding
This can only be done by yourself if you already contributed to this repo.
If your PR's code is not backward compatible with previous releases (which
should be avoided), add a message to the release notes by filing another PR:
https://github.com/openhab/openhab-distro/blob/main/distributions/openhab/src/main/resources/bin/update.lst
# Title
Provide a short summary in the *Title* above. It will show up in the release notes.
For example:
- [homematic] Improve communication with weak signal devices
- [timemachine][WIP] Initial contribution
# Description
Please give a few sentences describing the overall goals of the pull request.
Give enough details to make the improvement and changes of the PR understandable
to both developers and tech-savy users.
Please keep the following in mind:
- What is the classification of the PR, e.g. Bugfix, Improvement, Novel Addition, ... ?
- Did you describe the PRs motivation and goal?
- Did you provide a link to any prior discussion, e.g. an issue or community forum thread?
- Did you describe new features for the end user?
- Did you describe any noteworthy changes in usage for the end user?
- Was the documentation updated accordingly, e.g. the add-on README?
- Does your contribution follow the coding guidelines:
https://www.openhab.org/docs/developer/guidelines.html
- Did you check for any (relevant) issues from the static code analysis:
https://www.openhab.org/docs/developer/bindings/#include-the-binding-in-the-build
- Did you sign-off your work:
https://www.openhab.org/docs/developer/contributing.html#sign-your-work
# Testing
Your pull request will automatically be built and available under the following folder:
https://openhab.jfrog.io/ui/native/libs-pullrequest-local/org/openhab/addons/bundles/
It is a good practice to add a URL to your built JAR in this pull request description,
so it is easier for the community to test your Add-on.
If your pull request contains a new binding, it will likely take some time
before it is reviewed and processed by maintainers.
That said, consider submitting your Add-on in the Marketplace:
https://community.openhab.org/c/marketplace/69
Don't forget to submit a thread about your Add-on in the openHAB community:
https://community.openhab.org/c/add-ons
-->
Thank you,
Ciprian
lsiepel
(Leo)
January 8, 2024, 9:26pm
2
First of all i’m not familiair with modbus or sbus, so maybe these questions don’t make sense. Are these small differences worthwhile to create a seperate binding over a addition to modbus? Or maybe a sub binding org.openhab.binding.modbus.sbus and dervive/override from there?
Anyway, the current PR is a copy paste for many files and will need a readme.md with sbus specifics. Also filenames should be adapted.
Hey @lsiepel ,
These amendments were brought in three places:
The Modbus library: GitHub - cipianpascu/j2sbus: Enhanced Modbus library implemented in the Java programming language
The Modbus transport wrapper library for openhab
The Modbus binding
I duplicated the Modbus protocol because I couldn’t see a way to enhance the existing implementation.
If you have a better approach/proposal, let’s schedule a call to see what can be done.
As for the readme, yes, I will update it with S-Bus specific information.
Thank you,
Ciprian
Hi Sami ( @ssalonen ),
You are the specialist for the modbus binding, and the openhab core team asked you to review my changes related to the s-bus g4 protocol.
Please have a look and let me know if there are any additional action points that I should take. It is time-consuming for me to keep the branch in sync with the latest changes.
lsiepel suggested in the post above, to merge my changes directly into the modbus binding. What is your opinion? Would it be possible?
Kind regards,
Ciprian
system
(system)
Closed
March 2, 2024, 4:31am
5
This topic was automatically closed 41 days after the last reply. New replies are no longer allowed.