-
Notifications
You must be signed in to change notification settings - Fork 707
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
[INFO]: Supporting multiple versions of the same service (PRS_SOMEIPSD_00512). #797
Comments
Hi @KareemLMR, Yes, I have tested request_service() API for the same service-instances with different major versions, it works without any errors from the log. But I am not able to verify it using vsomeip server API since the offer_service() API doesn't support offering the same service-instances with different major versions within the same VLAN network which I mentioned in #809. Due to the limitation I have to ask our customer to verify it. It seems like our customer is using a 3rd party SOME/IP stack to deloy the services on different ECUs, which can support PRS_SOMEIPSD_00512. |
Hi @KareemLMR , Even we can specify the versions in some client side APIs like request_service(), subscribe() and register_availability_handler()/unregister_availability_handler(), there are still some APIs don't support passing the versions, which means the serviceID-instanceID-majVersion is not fully manageable independently. Thus I think we need to have below changes to fully support PRS_SOMEIPSD_00512 on client side to make sure the version is present in pair.
And request_event()/release_event() shall all add the major version as input parameter as below:
|
I tested the request_service() for the same instance but with different major versions, and it return error message as below: In my test I have two devices (A and B) in the network. First device B offers service 0x1234/5678 with majorVersion 0x12. Next device B request service 0x1234/5678 with majorVersion 0x12 and it works. Then device B request service 0x1234/5678 with majorVersion 0x22, it reported such error. From the source code, it seems like the majorVersion is not taken into consideration when maintaining the service instances available in the network. See void routing_manager_base::request_service() in routing_manager_base.cpp |
Hi @KareemLMR, From my investigation, It seems like the PRS_SOMEIPSD_00512 in SOME/IP SD Spec is totally not supported in vsomeip library, can you confirm ? |
Hello @zhaoxin39913 |
Hi @KareemLMR, I have made below changes per as your request:
And after change I have triggered "notify-sample" to offer the second service instance 0x1234/0x5678 with version 0xA (There is already a service instance 0x1234/0x5678 with version 0x12 offered by another device within the same network). And from the wireshark we can see that the first time notify-sample was successful to offer the service 0x1234/0x5678 version 0xA, but after call stop_offer() and then start_offer() in "notify-sample", it failed to offer the service again. Below are full log messages from notify-sample:
|
Ok, I see the log you added appeared. That's good. I saw what I wanted to see. Let me try a change at my side and send it to you to try when it is successful. |
Hi @KareemLMR , Any updates for your patch ? |
Hi @zhaoxin39913 |
Thanks @KareemLMR for sharing the update. |
You can try this change #829 I was able to offer 2 services with only different major versions through it but I need you to verify it. Here is the code examples: #include <vsomeip/vsomeip.hpp> #define SAMPLE_SERVICE_ID 0x1234 std::shared_ptr< vsomeip::application > app; int main() { service2.cpp: #include <vsomeip/vsomeip.hpp> #define SAMPLE_SERVICE_ID 0x1234 std::shared_ptr< vsomeip::application > app; int main() { Also for the json file, I supported adding "major" : "value" just as instance in your service definition so don't forget that. And in all other APIs you will pass unique instead of instance but also add major and minor if the API require it |
Hi @KareemLMR , Thanks for sharing the sample code. One question here, for client API request_service(), Do we still support setting ANY_MAJOR in unique_version_t parameter? Or we must pass a non-0xFF version ? And if ANY_MAYJOR is supported, how does the AvailabilityHandler get triggered if there are different majVersion instances in the network?
|
Hello @zhaoxin39913 |
Hi @KareemLMR , Currently I am only testing the client APIs which request different major versions for the same instance ID. It's working fine so far. I will continue to test those server APIs and keep you updated once my test is done. |
Hi @KareemLMR, Update some test results here for the patch(let's call it 3.5.3p). Now I confirmed the server app can offer two major version instances for the same service/instance ID successfully. And the client app can also detect different version instances when registering different major version availability handlers. However the basic request-response and event notification don't work in the patch. Here are my test setup: The server app running on device B offers service 0x238/0x1 with version 4.1 and always sends response with payload data "Version4.1: Reply Cnt=X" for any methods. In the meantime it periodically sends event 0x4465/0x8778 with payload data "Version4.1: Event Cnt=X“ every 5s. Test Case1 (No response and event received): Both client app and server app are running with vsomeip 3.5.3p. The client app log is as below:
Test Case 2 (Only response can be received): Client app is running with vsomeip 3.4.10, server app is running with vsomeip 3.5.3p. The client app log is as below:
Test Case 3 (Both reponse and event can be received) : Both client app and server app are running with vsomeip 3.4.10. The client app log is as below:
|
Hi @KareemLMR , Here is the my client app source code:
|
Hello @zhaoxin39913 |
Thank you for the feedback And don't forget to use unique_version_t in APIs that need them in the latest patch I just shared with you. |
Hi @KareemLMR , Thanks for your quick response. However, this is existing way we must support. In other word we need to keep the vsomeip API backward compatible in the patch, otherwise we will encounter regression issue, eg. COMMON API proxy/stub code is still using the existing vsomeip API. If we always introduce new API for existing feature, the COMMON API won't work, all related things need to be considered as well, then it will be a very big impact. |
I believe it is backward compatible as I didn't change any API in the last patch I shared. I said that last line just to confirm with you that you correctly used it from the older patch. You can even try it and tell me what you have found |
Thanks @KareemLMR , that's great! What I expect is the old APIs are still working in the network only has single major version instance. And we can use the new APIs to handle multiple major version instances. I will test the new patch and keep you updated. |
Hi @KareemLMR, Tested the second patch (3.5.3p2) you shared with client app(server app is still using 3.4.10), still see the same error log on client side:
git log :
|
Hello @zhaoxin39913 |
I tried to reproduce that issue of "[error] Routing info for remote service could not be found!". It only happened if I didn't set the major version in request : request_->set_major_version(major). When I set it, everything works fine. So I said you might forgot it as it was not in the old API but unfortunately this change in the API can't be avoided as the old API only takes service and instance but if we have multiple services with same instance but different major version, the send API won't work (how will it differentiate between them). So I believe it is impossible to implement this feature without this change, do you think of something different? We can negotiate about that if you have a different opinion, but can you try setting the major version as I told you just to check everything is fine? In my opinion, I believe this change is inevitable as I told you. However, I think I made some unnecessary changes to the rest of the APIs making the user forced to know the definition of unique_version_t. I think it is better to return the rest of the APIs like offer_service, request_service, ... and so on back to their original form of accepting instance and major and to add major version with a default value of 0 to the APIs that didn't take them originally like release_service, ..etc and then the unique_version to be constructed inside the vsomeip code NOT by the user so it will be 100% compatible with the old API. But I won't be able to do anything regarding the request_->set_major_version(major). It is unavoidable. If you agree with me. I can make these changes in another patchset but after you test the current version by setting major version in the request_ as I told you. |
Hi @KareemLMR, vsomeip already has message API set_interface_version() to do the same thing, which is duplicate with set_major_version(). Can you confirm ? |
Hi @KareemLMR , vsomeip already has message API set_interface_version() to do the same thing, which is duplicate with set_major_version(). Can you confirm ? |
I was thinking of that too. However, in the vsomeip code intervace_version was never used as major_version. They even have different types although both are uint8_t (interface_version_t and major_version_t). I tried to check the SOME/IP Specification Document, it is defined as "major version of the service interface" but in some requirments like this one attached below seems diffrent from the fixed service major version. It seems more like session id or protocol version so I am not sure so I didn't use it fearing it might not be known globally by vsomeip users that they are the same. However, anyway this will not affect my change much if we made sure both interface version and major version are the same. I will just replace the call of my newely defined method with this one in the code and it will be fine, if you confirm it works fine with my method set_major_version() |
vSomeip Version
v3.4.10
Boost Version
1.7.6
Environment
Ubuntu 20.04
Describe the bug
Hi vsomeip experts,
One issue I would like to check with you guys.
In order to support multiple versions of the same service (defined in SOME/IP SD spec PRS_SOMEIPSD_00512), there could be multiple services offered with the same serviceID and instanceID but different major versions within a network. Using API request_service(serviceId, instanceId, ANY_MAJOR) should be able to find all services with different major versions for a given serviceID/instanceID pair. Next are_available() API should be able to retrieve all those major versions.
The problem is how does availability_handler() get triggered when those services with different major versions are detected, since only the serviceID and instanceID is passing to the handler without major version.
This is one of our customer user case, what's the solution do you suggest to handle it in our application?
Reproduction Steps
N/A
Expected behaviour
N/A
Logs and Screenshots
N/A
The text was updated successfully, but these errors were encountered: