Skip to content

docs(sovd-api): add locks requirements and architecture - #395

Open
floroks wants to merge 2 commits into
mainfrom
feat/docs-locks
Open

docs(sovd-api): add locks requirements and architecture#395
floroks wants to merge 2 commits into
mainfrom
feat/docs-locks

Conversation

@floroks

@floroks floroks commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Summary

Add 9 requirements and architecture elements covering the SOVD lock API: CRUD endpoints, exclusivity, expiration, ECU and functional-group enforcement, vehicle-level blocking, priority preemption, defunct lock lifecycle, and HTTP 409 error semantics.

Also improve plantuml auto-detection in conf.py by falling back to shutil.which() before the hardcoded JAR path,
and fixes an error in the embedded files requirements (wrong path).

Checklist

  • I have tested my changes locally
  • I have added or updated documentation
  • I have linked related issues or discussions
  • I have added or updated tests

Related

Notes for Reviewers

@github-actions

github-actions Bot commented Jun 23, 2026

Copy link
Copy Markdown

📊 Coverage Report (unit + DoIP/CAN/mixed integration)

Total line coverage: 74.0%

Full build artifacts

@github-actions

github-actions Bot commented Jun 23, 2026

Copy link
Copy Markdown

⚠️ 1 CRAP regression(s) detected

↑ 1 regressed · ★ 0 new · ↔ 0 moved · ↓ 0 improved · 1768 unchanged · — 0 removed
✓ = clean, ▲ = moderate, ✗ = crappy; Δ = change since baseline; CC = cyclomatic complexity; Cov % = line coverage percentage

CRAP Δ CC Cov % Function Location
4.4 +0.4 4 70.8 UdsManager::start_variant_detection_for_ecus cda-comm-uds/src/variant.rs:43
🔥 Top hot spots above threshold
CRAP CC Cov % Function Location
600.0 24 0.0 EcuManager::map_table_struct_from_uds cda-core/src/diag_kernel/payload_decode.rs:670
306.0 17 0.0 dop_variant_to_schema cda-core/src/diag_kernel/schema.rs:410
272.0 16 0.0 MbedtlsError::name comm-mbedtls/mbedtls-rs/src/error.rs:67
240.0 15 0.0 TlsStream::poll_read comm-mbedtls/mbedtls-rs/src/async_stream.rs:381
210.0 14 0.0 DoipDiagGateway::send_functional cda-comm-doip/src/lib.rs:568
210.0 14 0.0 UdsManager::send_functional_group cda-comm-uds/src/functional_group.rs:182
156.0 12 0.0 update_mdd_uncompressed cda-database/src/mdd_data.rs:364
156.0 12 0.0 diag_service::functional_data_request cda-sovd/src/sovd/functions/functional_groups/data.rs:243
156.0 12 0.0 has_new_artifacts cda-storage/src/recovery.rs:265
110.0 10 0.0 DiagServiceError::from cda-comm-doip/src/connections.rs:72
110.0 10 0.0 comparams::executions::id::get cda-sovd/src/sovd/components/ecu/operations.rs:273
90.0 9 0.0 params_to_schema cda-core/src/diag_kernel/schema.rs:188
90.0 9 0.0 add_phys_constr_range cda-core/src/diag_kernel/schema.rs:306
90.0 9 0.0 EcuManager::map_table_key_from_uds cda-core/src/diag_kernel/payload_decode.rs:622
90.0 9 0.0 establish_tls_ecu_connection cda-comm-doip/src/ecu_connection.rs:406
90.0 9 0.0 create_tls_stream cda-comm-doip/src/ecu_connection.rs:501
90.0 9 0.0 remove_descriptions_recursive cda-sovd/src/sovd.rs:923
85.5 16 35.2 service::executions::ecu_reset_handler cda-sovd/src/sovd/components/ecu/operations.rs:1346
75.6 36 68.8 UdsManager::send_with_raw_payload cda-comm-uds/src/transport.rs:202
72.0 8 0.0 TlsStream::async_handshake comm-mbedtls/mbedtls-rs/src/async_stream.rs:237
72.0 8 0.0 UdsManager::set_functional_state cda-comm-uds/src/functional_group.rs:387
72.0 8 0.0 comparams::executions::id::put cda-sovd/src/sovd/components/ecu/operations.rs:401
70.6 10 15.4 DiagDataValue::try_into cda-core/src/diag_kernel.rs:211
56.0 7 0.0 UdsManager::send_functional_to_gateway cda-comm-uds/src/functional_group.rs:38
56.0 7 0.0 build_mode_response cda-sovd/src/sovd/functions/functional_groups/modes.rs:162

…and 37 more, see CI artifact for the full report.

@floroks
floroks force-pushed the feat/docs-locks branch 5 times, most recently from 945e6c6 to 2db5e1d Compare June 29, 2026 05:16
@floroks
floroks force-pushed the feat/docs-locks branch 2 times, most recently from f666059 to b89aae4 Compare July 7, 2026 07:45
@floroks
floroks marked this pull request as ready for review July 7, 2026 08:28
@floroks
floroks requested a review from a team as a code owner July 7, 2026 08:28
@github-actions github-actions Bot added the needs review Waiting for a review label Jul 7, 2026

@alexmohr alexmohr left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

couple of questions, mainly about the lock pre-emption :)

Comment thread docs/02_requirements/02_sovd.rst Outdated
:links: arch~sovd-api-lock-vehicle-blocking
:status: draft

When a vehicle lock is held by a client, any other client's attempt to acquire an ECU

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
When a vehicle lock is held by a client, any other client's attempt to acquire an ECU
When a vehicle lock is held by a client, any other clients attempt to acquire an ECU

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

would violate that claim.


.. req:: Lock Priority Preemption

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

q: Is this an optional mechanism that can be disabled or will we make this mandatory? Not sure this is wanted by every OEM.

@floroks floroks Aug 14, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, you get the option to implement this in a plugin (e.g. security), if you don't implement it, the standard is for the lock request to be denied

see `req~sovd-api-lock-priority:

If no vendor mechanism is configured, or if the mechanism does not grant preemption, conflicting POST requests are rejected with HTTP 423 as normal.

:links: arch~sovd-api-lock-defunct
:status: draft

When a lock is preempted by a higher-priority client it must transition to a

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

q: I do understand that certain operations like flashing should be done by clients with a high prio, so they won't get pre-empted. But what if a low prio client starts flashing too and then gets pre-empted. Do we abort to flash transfer or do we deny the pre-emption in this case? (I.e. the 'apply' of an runtime update is rejected when a flash transfer is running)
An alternative could be that certain operations are bound to a given lock priority, but that's probably something to implement in OEM side but OpenSOVD would have to provide the interfaces for this.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In practice it'll not just be a client, but a client + additional vendor specific additional information, which can be used to distinguish between different use-cases from the same client. Usually only a single client with a single specific additional data would be granted this type of prioritization which should then be the highest, but it's up to individual OEMs to define that.

- ``x_sovd2uds_broken_by`` -- the identity of the preempting client, as returned
by the vendor mechanism.
- ``x_sovd2uds_broken_at`` -- the ISO 8601 timestamp at which preemption occurred.
- ``x_sovd2uds_current_holder`` -- identity of the current lock holder

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

q: What's the usecase for broken by and current holder, in most cases they will be equal, except the In general, wouldn't it be nice then to return the current lock holder when locking a resource is rejected?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In most cases yes, but when a lock was only broken for a short time, and there is no current holder, the client can directly reattempt to get a lock. this would mostly be the case when someone is doing manual diagnostics (e.g. low frequency of request), and a quick higher prioritizes automated client doing a quick read from data.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

returning the current lockholder when locking is rejected can also be added, but it would then be an additional parameter in the locking response error parameters for the regular lock request

side-effects associated with acquiring and releasing a functional group lock.


.. arch:: Vehicle Lock Blocks Child Lock Acquisition

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

question: If a low prio client is holding a vehicle lock and the high prio client wants an ECU lock, will the low prio client be pre-empted anyway? Same question for functional groups, when the high prio client wants an ECU that is part of a low prio locked functional group.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lock preemption is only defined for vehicle locks, because it requires additional data to the client id. So a client can't request a higher prio lock for the ecu only, so it'll be rejected.

@github-actions github-actions Bot added changes requested Changes requested after a review and removed needs review Waiting for a review labels Aug 5, 2026
Add 9 requirements and 9 matching architecture elements covering the
SOVD lock API: CRUD endpoints, exclusivity, expiration, ECU and
functional-group enforcement, vehicle-level blocking, priority
preemption, defunct lock lifecycle, and HTTP 409 error semantics.

Also improve plantuml auto-detection in conf.py by falling back to
shutil.which() before the hardcoded JAR path.
The requirement referenced a non-existent endpoint path
/components/{ecu-name}/files/{file-name}. Updated to reflect the actual
bulk-data endpoints used by the implementation.

The architecture sequence diagram referenced a 'Diagnostic Kernel'
participant that does not exist. The handler calls FileManager directly;
replaced the participant and corrected call signatures (list/get) and
removed the non-existent variant parameter.
@github-actions github-actions Bot added needs review Waiting for a review and removed changes requested Changes requested after a review labels Aug 14, 2026
@github-actions

Copy link
Copy Markdown

Duplicate-code check passed - no duplication found in changed files.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs review Waiting for a review

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants