Fix wildcard regression in subscribe_current_values on v2 (closes #53)#59
Open
aki1770-del wants to merge 1 commit intoeclipse-kuksa:mainfrom
Open
Conversation
…ipse-kuksa#53) The v2 Subscribe RPC only accepts fully-qualified leaf paths, so passing a branch path like `['Vehicle']` — which v1 Subscribe accepted as a wildcard — now fails with NOT_FOUND since 0.5.1. This breaks existing user code that worked on 0.4.x. Restore the prior behavior by catching NOT_FOUND from v2 Subscribe and retrying with paths expanded via ListMetadata(root=path). Leaf paths continue on the fast path (no extra RPC). The UNIMPLEMENTED fallback to v1 Subscribe is preserved for old databrokers. - kuksa-client/kuksa_client/grpc/aio.py: async path, new _expand_v2_branch_paths helper. - kuksa-client/kuksa_client/grpc/__init__.py: sync mirror of the same change. - kuksa-client/tests/test_grpc.py: new test asserting branch-path expansion on NOT_FOUND. Trailing `.*` in paths (e.g. `Vehicle.Cabin.*`) is stripped before lookup so both the idiomatic v1 form and the explicit wildcard form work. Signed-off-by: Komada (aki1770-del) <aki1770@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Restores branch-path support in
subscribe_current_valueson the v2 code path.Since 0.5.1 preferred v2, calls like
client.subscribe_current_values(['Vehicle'])fail with
NOT_FOUND: the v2SubscribeRPC only accepts fully-qualified leafpaths. v1 Subscribe accepted branch paths natively, so this is a regression for
existing users.
Fixes #53.
How
NOT_FOUNDfrom the firstSubscribeattempt.ListMetadata(root=path)— the v2-native way toenumerate signals under a branch — and retry
Subscribewith the resultingleaves.
RPC in the common case.
.*(e.g.Vehicle.Cabin.*) is stripped before lookup so both thev1 idiom
['Vehicle']and the explicit wildcard form work.UNIMPLEMENTED→ v1 fallback is preserved for old databrokers.Scope
Strictly
subscribe_current_values(sync + async).subscribe_target_valueshas the same shape but is out of scope here — happy to file a symmetric fix in
a follow-up if the approach looks right.
Existing behavior audit
v2_subscribeimplementation:kuksa-client/kuksa_client/grpc/aio.py:537—passes
pathsdirectly intoSubscribeRequest; no client-side expansion._prepare_v2_list_metadata_request:kuksa-client/kuksa_client/grpc/__init__.py:907already builds the
ListMetadatarequest shape. Reused here.val.protosemantics forListMetadata: "Shall correspond to a VSS branch,e.g.
Vehicle,Vehicle.Cabin. Metadata for all signals under that branchwill be returned." (
kuksa/val/v2/val.proto)get_metadatastill works with wildcards (per the issue reporter) — whichis why this is a clear regression from the v2 switch, not a missing feature.
Prior art / related
subscribe_target_valuesto use v2 API (BjoernAtBosch, merged)set_current_valuesusingkuksa.val.v2API (BjoernAtBosch, merged)Tests
test_subscribe_current_values— unchanged, still passing (leaf-path fast path)test_subscribe_current_values_branch_path_expansion— new; asserts that onNOT_FOUND the client retries
v2_subscribewith expanded leavesNot tested (would appreciate reviewer input)
happy to add an integration test if the project has a pattern for that.