-
Notifications
You must be signed in to change notification settings - Fork 7
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Generalize the lib's binding API #145
base: master
Are you sure you want to change the base?
Conversation
…java into feature/more-bindings
… in the response)
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## master #145 +/- ##
============================================
- Coverage 87.53% 85.46% -2.07%
- Complexity 504 547 +43
============================================
Files 43 53 +10
Lines 1901 2071 +170
Branches 254 283 +29
============================================
+ Hits 1664 1770 +106
- Misses 161 220 +59
- Partials 76 81 +5
☔ View full report in Codecov by Sentry. |
Hi @vcharpenay, I have this PR in mind to be looked this week (also see how to increase coverage to target). Would you consider the branch stable enough to be merged with main? |
We've used this version of the lib in the ai4industry summer school last week. I also used it in another project. The binding API has been quite stable since then, and we haven't found bugs in the HTTP binding so far. I haven't tried to integrate the CoAP binding, though. I don't have a test bed for that. |
@vcharpenay is the README up to date? |
The code base to merge has a generic API for protocol bindings. See the entry point
bindings.ProtocolBindings
. Clients can initiate operations without knowledge of the underlying protocol (HTTP, CoAP or anything else), as suggested in #72.HTTP and CoAP bindings have been updated accordingly. Other protocol bindings are available for OPC-UA and for ROS.
The README isn't up to date regarding e.g.
TdHttpRequest
. It should be updated before merging.