-
Notifications
You must be signed in to change notification settings - Fork 20
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
Subscribe by id Integer32 implementation #81
Subscribe by id Integer32 implementation #81
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## feature/databroker-api-v2 #81 +/- ##
=============================================================
+ Coverage 54.99% 55.33% +0.33%
=============================================================
Files 33 33
Lines 14525 14677 +152
=============================================================
+ Hits 7988 8121 +133
- Misses 6537 6556 +19 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good not tested yet. Just one comment. Will try to test and start today as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is PR is changing way more than necessary. Databroker already supports the concept of using id
instead path
to refer to signals.
This is for example used by providers in the sdv.databroker.v1
interface.
To add support for this in kuksa.val.v2
, you should in theory only need to change stuff under grpc/kuksa_val_v2
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
2aecd5c
to
7b47ed9
Compare
8a605af
to
f1b9b71
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
@argerus - can you check if you are happy with it now |
a655241
to
a8df2f6
Compare
d0424e9
into
eclipse-kuksa:feature/databroker-api-v2
A variation of the Subscribe method which improves performance by using an Integer32 ID instead of a string for signals parameter.