Skip to content
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

SOME/IP specification cleanup #165

Merged
merged 3 commits into from
Jun 13, 2024

Conversation

int0x27
Copy link
Contributor

@int0x27 int0x27 commented May 27, 2024

The following change is a reboot of the SOME/IP specifications in an attempt to drastically simplify to show how we map up-l1 messages to SOME/IP messages and uSubscription messages to SOME/IP-SD messages. the older specifications had a lot of uProtocol version 1.3.6 concepts (cloudevents) that were rather complicating the situation.

(based on PR-117, that can't be merged at the moment)

Fixes #93

up-l1/someip.adoc Outdated Show resolved Hide resolved
|===
| *UUri Portion* | *SOME/IP Field* | *Description*
a| `authority_name` | IP address & port
a| IP address & port (destination endpoint) of the mDevice.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we need to elaborate more on this portion. The URI can be either a host or IP address. If the authority_name is host, we need to then resolve to an IP address in the transport implementation.

Copy link
Contributor

Choose a reason for hiding this comment

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

Curious to learn more here too.

Copy link
Contributor Author

@int0x27 int0x27 May 28, 2024

Choose a reason for hiding this comment

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

vsomeip gets the required endpoint information (via service discovery or static json configuration), but they have decided to leave it private for the implementation, so you have a better abstraction of SOME/IP services/instances/methods available in the network.

e.g. it is possible to have multiple endpoints (hosts) offering the same service, but with different instance or version. vsomeip router aggregates all discovered service/instance information and maps the endpoint to service/instance/major_ver/minor_ver/[method | eventgrup/event]
API requires service and instance as minimum.

The only way I see to provide the mDevice IP:port in authority_name is to have it pre-configured (mapped to service/instance pair).

If "host" means any string, it could be created from service/instance pair, e.g. <service>.<instance>v<version>, but then that same information is present also in ue_id, ue_version_major

a| `authority_name` | IP address & port
a| IP address & port (destination endpoint) of the mDevice.

NOTE: `vsomeip` hides IP endpoint information and uses `Service ID`/`Instance ID` instead.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you need to elaborate more what is meant here. A service (and instance) is deployed to a given ECU device that has an IP address, I don't think it is hidden per say.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Although it's not hidden by the protocol, it is hidden by the implementation. You can't work directly with the endpoints, so they have abstracted them by service/instance/..

up-l1/someip.adoc Outdated Show resolved Hide resolved
a| `UMESSAGE_TYPE_RESPONSE` a| `RESPONSE` or `ERROR` | Responses or error has occurred while attempting to deliver the message or a service
has thrown an exception

a| `UMESSAGE_TYPE_NOTIFICATION` a| `NOTIFICATION` | Notification
Copy link
Contributor

Choose a reason for hiding this comment

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

Per our chat, this is a different type of message that is a request with no response correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

UMESSAGE_TYPE_NOTIFICATION was previously mapped to REQUEST_NO_RESPONSE, which seems wrong.

I can't tell whether to use UMESSAGE_TYPE_PUBLISH or UMESSAGE_TYPE_NOTIFICATION for incoming SOME/IP events. e.g. if we have a subscriber to the specific service/instance/event, it should be a UMESSAGE_TYPE_NOTIFICATION, if no subscribers are found send it as UMESSAGE_TYPE_PUBLISH?

up-l1/someip.adoc Outdated Show resolved Hide resolved
When sending auto-generated *SOME/IP* REQUEST messages:

* *MUST* cache the message's `Request ID` to correlate with the RESPONSE message.
* Underlying *SOME/IP* library *SHOULD* handle `Request ID` updating.
Copy link
Contributor

Choose a reason for hiding this comment

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

@int0x27 I don't understand this requirement, can you please explain what you mean here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I mean let the library do the Request ID manipulation.

The transport just needs to send() the request and cache the updated Request ID (as there are multiple cases, whether to increment Session ID or not).
Sending notifications also has automatic Request ID assignment.

// removed some lines about setting Client ID to 0 on events / incrementing the Session ID, etc.

up-l1/someip.adoc Outdated Show resolved Hide resolved

|===

NOTE: `UUri.authority_name` *MAY* be translated to/from IPv4 (and/or IPv6) Endpoint Option of the *SOME/IP-SD* message, although in `vsomeip` this is not available in the API (e.g. each discovered Endpoint maps to `Service ID`/`Instance ID`/`Major Version`/`Minor Version`).
Copy link
Contributor

Choose a reason for hiding this comment

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

This is where we need to chat, I'm not sure how that would work. How do we know that service_id+instance_id maps to a specific IP address and port, is that static configuration that would be loaded into the uTransport implementation library?

Copy link
Contributor

@PLeVasseur PLeVasseur May 27, 2024

Choose a reason for hiding this comment

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

Also very curious about this point.

I don't know vsomeip all that well, but from what I could see it seems to rely on static configuration of IP addresses and I couldn't figure out reading the vsomeip header how header contents tied back to an IP address.

If the answer is as you wrote and there is no IP address or logical name contained within the vsomeip header, how do we come up with this?

Must we then read that same vsomeip configuration to be able to correlate service and instance IDs with the IP address?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Some other SOME/IP implementations, require you to specify all the local and remote endpoints in their static configuration along with their service/instance/methods, etc.

For vsomeip it can be also static (if service discovery is disabled) or auto-populated from service discovery.
Consider you want to use {service:0x1234, instance:2, method:0x0042, major_ver:1}

  • The app registers on_available(service, instance, is_available) callback, possibly with ANY_SERVICE, ANY_INSTANCE artguments.
  • App calls app->request_service(0x1234, ANY_INSTANCE, 1, ANY_MINOR)
  • This generates Find Service Entry SOME/IP message [PRS_SOMEIPSD_00350]
    and all mDevice endpoints must return their 0x1234 service endponits (e.g. hostA provides instance 1, hostB provides instance 2)
  • App receives on_available(0x1234, 1, true) and on_available(0x1234, 2, true) and then it can communicate with the choosen instance
  • e.g. send a message to configured instance 2:
std::shared_ptr<vsomeip::message> rq = vsomeip::runtime::get()->create_request(use_tcp_);
rq->set_service(0x1234);
rq->set_instance(2);
rq->set_method(0x0042);
rq->set_interface_version(1); // major_ver
std::shared_ptr<vsomeip::payload> pl = vsomeip::runtime::get()->create_payload();
// serialize payload for method_id
rq->set_payload(pl);
app_->send(rq);
  • vsomeip finds the discovered service/instance(2) and sends the request to HostB endpoint (app does not care about which host is providing it)
  • You can also set rq->set_instance(1) and it will be sent to the other hostA

a| `SubscriptionResponse`
a| The message is used to acknowledge a failed subscription request.

* SubscriptionStatus.code *SHALL* be set to the corresponding error code per the <<commstatus-mapping>> table
Copy link
Contributor

Choose a reason for hiding this comment

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

This is changing in #163, it might be better to just state that ,the error codes below will be returned and not through SubscriptionStatus.code (it will be in commstatus of UAttributes)

Copy link
Contributor

@PLeVasseur PLeVasseur left a comment

Choose a reason for hiding this comment

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

Hi @int0x27 -- I don't know much about vsomeip yet.

I'm still a bit unclear on how the source and sink UUris map in.

Could I ask for some elaboration on how you see the source and sink UUris lining up with the SOME/IP header?

I'm looking for something akin to what was done for Zenoh or MQTT5.

I'd essentially like the kind of clarity laid out in those documents on how to map from the uP-L1 registerListener API down into the vsomeip header.

This kind of information will be essential when a uStreamer is fleshed out, as we'll need to be able to say things like:

"Give me all point-to-point (Request, Response, Notification) messages with a source authority of X, so that I can route them to sink authority Y"

and

"Give me Publish messages from topics A, B, and C, since I will need to bring those onto this host for dissemination to uEntities Foo: {A, B} and Bar: {C}"

NOTE: `vsomeip` hides IP endpoint information and uses `Service ID`/`Instance ID` instead.

a| `ue_id` a| `Instance ID` \| `Service ID`
a| `Service ID` *MUST* be encoded in the lower 16 bits of `resource_id`.

Choose a reason for hiding this comment

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

As per my understanding,
Service ID MUST be encoded in the lower 16 bits of ue_id and not 'resource_id'.
Please confirm


Maps to `Major Version` for the `Service ID`. If set, it *SHOULD* be passed to underlying *SOME/IP* library to override it's default _Service Major Version_

a| `resource_id` a| `Method ID`/`Event ID` | Identifier of the method/event placed in the
Copy link

@pranavishere2 pranavishere2 May 31, 2024

Choose a reason for hiding this comment

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

As per my understanding,
uProtocol resource_id is mapped to method_id in someip header for RPC usecase and
uProtocol resource_id is mapped to event ID in someip header for Pub Sub usecase

Question: How are we going to map event group id in vsomeip ? If we assign event_id same as event_groupID then we may have a problem. We faced this issue few weeks ago where there was a use-case of different event_id and event_group ID

FYI In someip world,
At client side,
Event ID and event group ID both needs to be provided while calling request_event API and "event groupID" needs to be provided again for subscribe API.

At server side,
same event ID and event group ID needs to be provided while calling offer_event API

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My opinion is still in favor of using Event ID for both PUBLISH and SubscriptionRequest.topic.resource_id.
This needs additional configuration for the transport (mapping Service/Instance/Event to a set of eventgroup IDs), but it allows a lot more flexibility in supporting ECUs.
The other option: forcing ECUs to have EventID=EventGroupID is a lot more restrictive for the HW and not OSS friendly. This can be assumed as "default" option - if no mapping is provided per event, assume EventID=EventGroupID


NOTE: Explicitly setting `Instance ID` to `0x0001` *SHOULD* be avoided as it increases the message size.

a| `ue_version_major` a| `Interface Version`
Copy link

@pranavishere2 pranavishere2 May 31, 2024

Choose a reason for hiding this comment

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

As per my understanding,
uProtocol "ue_version_major" is mapped to Service Major Version in someip. What about minor version?
In someip world, minor version is also required at the time of offer service at server side and request service at client side. We will have a problem when mE would try to offer a service with different minor version and Streamer(up-client-vsomeip-cpp) will need to request the same service with that specific minor version.
I think, we need a mapping of ue_version_minor to map it to vsomeip Service Minor version as well.
OR
Are we suppose to use ANY_MINOR as a default while invoking "request_service" vsomeip API call and that way no mapping is needed?
Please confirm

Copy link
Contributor Author

Choose a reason for hiding this comment

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

minor version is needed indeed , if omitted vsomeip assumes vsomeip::DEFAULT_VERSION=0x00000000.
The same applies for major version (assumed 0x00, if not provided)
As minor is not available in uProtocol, I suppose it should be a transport configuration (especially for offered services).
For requested services we can get away with vsomeip::ANY_MINOR

The transport MUST know in advance which services/instances/major/minor and events it MAY interact with and pre-register them in the vsomeip router. It could be possible to request the event in e.g. on_available handler, but that may block the dispatcher thread for too long and you'll get vsomeip warnings.

Copy link
Contributor

@stevenhartley stevenhartley left a comment

Choose a reason for hiding this comment

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

please see feedback comments

up-l1/someip.adoc Outdated Show resolved Hide resolved
up-l1/someip.adoc Outdated Show resolved Hide resolved
up-l1/someip.adoc Outdated Show resolved Hide resolved
up-l1/someip.adoc Outdated Show resolved Hide resolved
up-l1/someip.adoc Show resolved Hide resolved
up-l1/someip.adoc Show resolved Hide resolved
@stevenhartley
Copy link
Contributor

@int0x27 I think we are OK to merge, next week we can add specific mappings of the uTransport API given the work that Peter has done on up-transport-vsomeip-rust.

int0x27 added 3 commits June 7, 2024 20:31
based on PR-117

Signed-off-by: Ivan Kirchev <78968972+int0x27@users.noreply.github.com>
fixed links in README.adoc

Signed-off-by: Ivan Kirchev <78968972+int0x27@users.noreply.github.com>
also removed short `ttl` handling suggestion

Signed-off-by: Ivan Kirchev <78968972+int0x27@users.noreply.github.com>
@int0x27 int0x27 force-pushed the someip-spec-update branch from 82262bb to 93fa446 Compare June 7, 2024 17:32
@int0x27 int0x27 marked this pull request as ready for review June 7, 2024 17:37
@int0x27
Copy link
Contributor Author

int0x27 commented Jun 7, 2024

OK, just rebased on main, squashed some commits

Copy link
Contributor

@stevenhartley stevenhartley left a comment

Choose a reason for hiding this comment

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

LGTM for now, we'll add to it more for uTransport specifics next week

@stevenhartley stevenhartley merged commit 32c60d7 into eclipse-uprotocol:main Jun 13, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update SOME/IP specification to uProtocol 1.5.x
4 participants