feat(someip): add server/provider side (offer services, answer RPC, publish events) - #913
feat(someip): add server/provider side (offer services, answer RPC, publish events)#913kirkbrauer wants to merge 1 commit into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
🚧 Files skipped from review as they are similar to previous changes (6)
📝 WalkthroughWalkthroughThe SOME/IP driver now supports provider/server mode, including service offering, canned RPC responses, event registration and publication, field updates, introspection, lifecycle management, client wrappers, tests, documentation, and an example exporter configuration. ChangesSOME/IP provider support
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant SomeIpDriverClient
participant SomeIp
participant OsipServer
Client->>SomeIpDriverClient: configure provider behavior
SomeIpDriverClient->>SomeIp: call server API
SomeIp->>OsipServer: start, offer service, register handlers
OsipServer-->>SomeIp: deliver RPC or subscriber request
SomeIp->>OsipServer: return canned response or publish event
Possibly related issues
Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Checkov (3.3.8)python/packages/jumpstarter-driver-someip/examples/exporter.yamlTraceback (most recent call last): Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@vtz — as the original author of this SOME/IP driver, your review would be much appreciated. This adds the server/provider side (offer services via SD, answer RPC with canned responses, publish events) on top of your client-side implementation, wrapping the |
…ublish events) The SOME/IP driver was client-only. opensomeip already ships a full server-side API (SomeIpServer / SdServer / RpcServer / EventPublisher), so expose it through the driver so a Jumpstarter exporter can act as a simulated ECU that a device-under-test's SOME/IP client talks to. New exported verbs (driver + client): - start_server / stop_server: server lifecycle (lazily started on first offer) - offer_service / stop_offer_service / list_offered_services: SD offering - set_method_response / clear_method_response: canned RPC responses. RPC handlers run in the exporter process and cannot call back to the Jumpstarter client per request, so responses are configured declaratively per (service_id, method_id); this maps cleanly onto getter-style methods. - register_event / publish_event / set_field: event group notifications and cached field events. Adds a SomeIpOfferedService model, README "Server / Provider" usage section, a provider exporter example, and 13 server-side unit tests (83 passed). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
756f728 to
f07ee6b
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
python/packages/jumpstarter-driver-someip/jumpstarter_driver_someip/driver.py (1)
359-369: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valuePrefer
try/except ValueErrorover the enum internal_value2member_map_.
ReturnCode._value2member_map_is a CPython enum implementation detail. Constructing the enum in atry/exceptis the documented idiom and is robust to future enum internals.♻️ Optional refactor
- payload, return_code = self._method_responses.get(key, (b"", int(ReturnCode.E_OK))) - rc = ReturnCode(return_code) if return_code in ReturnCode._value2member_map_ else ReturnCode.E_NOT_OK + payload, return_code = self._method_responses.get(key, (b"", int(ReturnCode.E_OK))) + try: + rc = ReturnCode(return_code) + except ValueError: + rc = ReturnCode.E_NOT_OK🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@python/packages/jumpstarter-driver-someip/jumpstarter_driver_someip/driver.py` around lines 359 - 369, Update the ReturnCode conversion in handler to construct ReturnCode(return_code) inside a try block and catch ValueError to fall back to ReturnCode.E_NOT_OK. Remove the direct use of ReturnCode._value2member_map_, while preserving the existing response/error message type behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In
`@python/packages/jumpstarter-driver-someip/jumpstarter_driver_someip/driver.py`:
- Around line 359-369: Update the ReturnCode conversion in handler to construct
ReturnCode(return_code) inside a try block and catch ValueError to fall back to
ReturnCode.E_NOT_OK. Remove the direct use of ReturnCode._value2member_map_,
while preserving the existing response/error message type behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 5d0c1305-d16a-4d5a-83aa-e3bbee1e30f0
📒 Files selected for processing (6)
python/packages/jumpstarter-driver-someip/README.mdpython/packages/jumpstarter-driver-someip/examples/exporter.yamlpython/packages/jumpstarter-driver-someip/jumpstarter_driver_someip/client.pypython/packages/jumpstarter-driver-someip/jumpstarter_driver_someip/common.pypython/packages/jumpstarter-driver-someip/jumpstarter_driver_someip/driver.pypython/packages/jumpstarter-driver-someip/jumpstarter_driver_someip/driver_test.py
|
I will remove my review to let @vtz come and review. |
vtz
left a comment
vtz
left a comment
There was a problem hiding this comment.
Thanks for this. Nice addition. Exposing the provider side so an exporter can act as a simulated ECU fits Open SOME/IP and opensomeip well, and one driver ≈ one ECU is the right Jumpstarter shape.
Inline comments cover the small cleanup/test nits. Separately, worth noting for later (not blockers): Fire&Forget and the full Field getter/setter/notifier model from the spec aren’t in scope here (canned RPC + set_field/events cover the usual simulator path).
Great work!
| return SomeIpMessageResponse.model_validate( | ||
| self.call("rpc_call", service_id, method_id, msg, timeout) | ||
| ) | ||
| return SomeIpMessageResponse.model_validate(self.call("rpc_call", service_id, method_id, msg, timeout)) |
There was a problem hiding this comment.
This is just a formatting change, do we want to change it?
| return SomeIpMessageResponse.model_validate( | ||
| self.call("receive_message", timeout) | ||
| ) | ||
| return SomeIpMessageResponse.model_validate(self.call("receive_message", timeout)) |
| return SomeIpEventNotification.model_validate( | ||
| self.call("receive_event", timeout) | ||
| ) | ||
| return SomeIpEventNotification.model_validate(self.call("receive_event", timeout)) |
| def offer_service( | ||
| self, | ||
| service_id: int, | ||
| instance_id: int = 0x0001, |
There was a problem hiding this comment.
SOME/IP reserves Instance ID 0x0000 and uses 0xFFFF as “all instances.” Neither should be offered. Consider rejecting those values in offer_service (and optionally on SomeIpOfferedService.instance_id).
| def stop_server(self) -> None: | ||
| """Stop the SOME/IP server, withdrawing all offers.""" | ||
| if self._osip_server is not None: | ||
| try: | ||
| self._osip_server.stop() | ||
| except Exception: | ||
| logger.warning("failed to stop opensomeip server", exc_info=True) | ||
| self._osip_server = None | ||
| self._registered_methods.clear() | ||
| self._registered_events.clear() |
There was a problem hiding this comment.
_registered_methods / _registered_events are cleared, but _method_responses is not. After stop + restart, stale payloads remain until overwritten, while handlers are only re-registered on the next set_method_response. Please clear _method_responses too, or document that config intentionally survives stop_server.
|
|
||
| @export | ||
| @validate_call(validate_return=True) | ||
| def publish_event(self, service_id: int, event_id: int, payload: SomeIpPayload) -> None: |
There was a problem hiding this comment.
service_id is accepted here but never forwarded (opensomeip only takes event_id). Please document that it’s unused for API symmetry, or remove it so callers aren’t misled.
| # ========================================================================= | ||
| # Server / provider side tests | ||
| # | ||
| # The server verbs let the endpoint ACT AS an ECU (offer services, answer | ||
| # RPC with canned responses, publish events). We patch the opensomeip | ||
| # SomeIpServer so these run without real networking, and assert the driver | ||
| # drives the underlying server API correctly. | ||
| # ========================================================================= |
There was a problem hiding this comment.
Server coverage is only unit tests against _FakeOsipServer. Please add at least one stateful/loopback-style test (offer + set_method_response + client RPC), similar in spirit to the existing client stateful suite. That would better cover the simulated-ECU use case.
3 participants
The SOME/IP driver is currently client-only.
opensomeipalready ships a full server-side API (SomeIpServer/SdServer/RpcServer/EventPublisher), so this exposes it through the driver so a Jumpstarter exporter can act as a simulated ECU that a device-under-test's SOME/IP client talks to.New exported verbs (driver + client)
start_server/stop_server— server lifecycle (lazily started on first offer)offer_service/stop_offer_service/list_offered_services— SD offeringset_method_response/clear_method_response— canned RPC responsesregister_event/publish_event/set_field— event-group notifications and cached field eventsDesign note
RPC handlers run inside the exporter process (opensomeip receive thread) and cannot call back to the Jumpstarter client per request, so responses are configured declaratively per
(service_id, method_id)rather than via a per-request callback. This maps naturally onto getter-style SOME/IP methods, updating the response changes what a client reads without re-registration.Also included
SomeIpOfferedServicemodel for server-side introspectionopensomeip.SomeIpServer); full package suite: 83 passed, 2 skippedThe client side is unchanged; all additions are additive.