[POC] feat: add proxy-core crate with shared types, MDD service resolver, and workspace setup#11
[POC] feat: add proxy-core crate with shared types, MDD service resolver, and workspace setup#11arvindr19 wants to merge 12 commits into
Conversation
…nd workspace setup Signed-off-by: Arvind Rathod <Arvind.RA.Rathod@bti.bmwgroup.com>
de4b24b to
a5837f9
Compare
e4f0f6d to
7117777
Compare
7117777 to
da5fa2c
Compare
Signed-off-by: Arvind Rathod <Arvind.RA.Rathod@bti.bmwgroup.com>
darkwisebear
left a comment
There was a problem hiding this comment.
The service_resolver.rs file is just too big. Please split this into smaller units and add a reference to some class diagram or the like so that I as a human being do not have to reverse engineer what it does.
9729efb to
2b5f19b
Compare
Signed-off-by: Arvind Rathod <Arvind.RA.Rathod@bti.bmwgroup.com>
2b5f19b to
b0c39a0
Compare
darkwisebear
left a comment
There was a problem hiding this comment.
Haven't been able to finish the review, but there is already some material to work on.
ce1e93b to
529c1b4
Compare
Signed-off-by: Arvind Rathod <Arvind.RA.Rathod@bti.bmwgroup.com>
529c1b4 to
b1ef0eb
Compare
- Add UdsResponse<'a> wrapper with sid()/is_negative()/nrc() accessors
- is_negative_response/get_nrc delegate to UdsResponse — no more raw byte indexing
- Add ResolvedService { name, params } struct; replace (String, Map) tuple in
resolve_read_service / resolve_write_service return types
- Split resolve_service_did into build_did_candidates + match_candidate_did
- or_else + match pattern in activate_base_variant (replaces nested if/else)
- Add RwLock doc comment explaining write-reservation intent
- Remove redundant HashSet alongside Vec in candidate dedup
- Add #[cfg(test)] modules to resolve.rs, metadata.rs, response.rs
(6 + 9 + 12 new unit tests; 63 total in proxy-core)
bharatGoswami8
left a comment
There was a problem hiding this comment.
@arvindr19 , I’ve reviewed service_resolver and suggest refactoring it into feature-specific types.
For example, we could move metadata into its own type and then used as a component of the primary ServiceResolver or as a specialized internal type. This approach would significantly improve the code's modularity and structural clarity.
Also please add justification for clippy supresseion.
Signed-off-by: Arvind Rathod <Arvind.RA.Rathod@bti.bmwgroup.com>
Thank you for the review, |
Signed-off-by: Arvind Rathod <Arvind.RA.Rathod@bti.bmwgroup.com>
bc6b669 to
7d6e2bf
Compare
Signed-off-by: Arvind Rathod <Arvind.RA.Rathod@bti.bmwgroup.com>
|
|
||
| /// Return a [`DidResolver`] for DID-to-service resolution. | ||
| #[must_use] | ||
| pub fn did_resolver(&self) -> DidResolver { |
There was a problem hiding this comment.
do we want to give different type to user?
How about ServiceResolver will work as centralize or user consumer instance and it will maintain all this type as a private internally, so with that all the public APIs can be called with ServiceResolver instance.
There was a problem hiding this comment.
updated, now ServiceResolver has 3 sub components and only necessary APIs made pub.
- Add UdsResponse<'a> wrapper with sid()/is_negative()/nrc() accessors
- is_negative_response/get_nrc delegate to UdsResponse — no more raw byte indexing
- Add ResolvedService { name, params } struct; replace (String, Map) tuple in
resolve_read_service / resolve_write_service return types
- Split resolve_service_did into build_did_candidates + match_candidate_did
- or_else + match pattern in activate_base_variant (replaces nested if/else)
- Add RwLock doc comment explaining write-reservation intent
- Remove redundant HashSet alongside Vec in candidate dedup
- Add #[cfg(test)] modules to resolve.rs, metadata.rs, response.rs
(6 + 9 + 12 new unit tests; 63 total in proxy-core)
Signed-off-by: Arvind Rathod <Arvind.RA.Rathod@bti.bmwgroup.com>
- Add UdsResponse<'a> wrapper with sid()/is_negative()/nrc() accessors
- is_negative_response/get_nrc delegate to UdsResponse — no more raw byte indexing
- Add ResolvedService { name, params } struct; replace (String, Map) tuple in
resolve_read_service / resolve_write_service return types
- Split resolve_service_did into build_did_candidates + match_candidate_did
- or_else + match pattern in activate_base_variant (replaces nested if/else)
- Add RwLock doc comment explaining write-reservation intent
- Remove redundant HashSet alongside Vec in candidate dedup
- Add #[cfg(test)] modules to resolve.rs, metadata.rs, response.rs
(6 + 9 + 12 new unit tests; 63 total in proxy-core)
- Add UdsResponse<'a> wrapper with sid()/is_negative()/nrc() accessors
- is_negative_response/get_nrc delegate to UdsResponse — no more raw byte indexing
- Add ResolvedService { name, params } struct; replace (String, Map) tuple in
resolve_read_service / resolve_write_service return types
- Split resolve_service_did into build_did_candidates + match_candidate_did
- or_else + match pattern in activate_base_variant (replaces nested if/else)
- Add RwLock doc comment explaining write-reservation intent
- Remove redundant HashSet alongside Vec in candidate dedup
- Add #[cfg(test)] modules to resolve.rs, metadata.rs, response.rs
(6 + 9 + 12 new unit tests; 63 total in proxy-core)
There was a problem hiding this comment.
question: So this is more a general question than it is with this particular change. Would it help to exrtact the testing utils, like the ECU sim, testcontainer, odx generation etc. into a separate repo cda and proxy can just use as submodule?
Signed-off-by: Arvind Rathod <Arvind.RA.Rathod@bti.bmwgroup.com>
629276e to
1bcaf23
Compare
- Add UdsResponse<'a> wrapper with sid()/is_negative()/nrc() accessors
- is_negative_response/get_nrc delegate to UdsResponse — no more raw byte indexing
- Add ResolvedService { name, params } struct; replace (String, Map) tuple in
resolve_read_service / resolve_write_service return types
- Split resolve_service_did into build_did_candidates + match_candidate_did
- or_else + match pattern in activate_base_variant (replaces nested if/else)
- Add RwLock doc comment explaining write-reservation intent
- Remove redundant HashSet alongside Vec in candidate dedup
- Add #[cfg(test)] modules to resolve.rs, metadata.rs, response.rs
(6 + 9 + 12 new unit tests; 63 total in proxy-core)
- Add UdsResponse<'a> wrapper with sid()/is_negative()/nrc() accessors
- is_negative_response/get_nrc delegate to UdsResponse — no more raw byte indexing
- Add ResolvedService { name, params } struct; replace (String, Map) tuple in
resolve_read_service / resolve_write_service return types
- Split resolve_service_did into build_did_candidates + match_candidate_did
- or_else + match pattern in activate_base_variant (replaces nested if/else)
- Add RwLock doc comment explaining write-reservation intent
- Remove redundant HashSet alongside Vec in candidate dedup
- Add #[cfg(test)] modules to resolve.rs, metadata.rs, response.rs
(6 + 9 + 12 new unit tests; 63 total in proxy-core)
b33b7bf to
5750b9b
Compare
Summary
Restructures the repository into a Cargo workspace and introduces the
proxy-corefoundation crate.Changes
Workspace migration: Converts the single-crate layout to a multi-crate workspace with
proxy-core,proxy-doip,proxy-sovd,proxy-mainmembers (follow up PRs [POC] feat: add proxy-sovd crate with SOVD REST client and UDS <->SOVD mapper #12 , [POC] feat: add proxy-doip crate — DoIP transport layer (temporary, RDBI/WDBI only) #13 & [POC] feat: add proxy-main binary, FLXC1000 test client #14 ). CDA dependencies pinned to eclipse-opensovd/classic-diagnostic-adapter@cf7fd9c1.proxy-core:Config/ServerConfig/SovdConfig/EcuConfig--> TOML-driven configurationProxyError/UdsError/SovdError/Nrc--> structured error hierarchy with ISO 14229-1 NRC codesServiceResolverexposes three composable components:DidResolver- DID-to-service resolutionResolvedServicewith matched service name and decoded request parametersresolve_read_service()/resolve_write_service()methods for READ/WRITE servicesEcuManagerfor MDD lookupsResponseEncoder- UDS response encodingMetadataProvider- MDD parameter metadata queriesImplementation Details
resolve.rs- DID resolution logic with prefix lookup and request parameter extractionresponse.rs- Response encoding with support for computed/physical/value parametersmetadata.rs- Parameter metadata queries and inspectionuds_helpers.rs- Low-level UDS byte manipulation and matching utilitiesconfig.toml: Default proxy configurationtestcontainer/mdd/FLXC1000.mdd: Open-source sample MDD for testingScope
Parse and validate UDS requests and responses using MDD structures.
Current implementation covers ReadDataByIdentifier (0x22) and WriteDataByIdentifier (0x2E) services only.
Current Approach
Checklist
Related
Notes for Reviewers