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

Adding error section to existing method inserts error byte into first byte of reply body rather than last byte of SOME/IP header #49

Open
srl100 opened this issue Apr 4, 2024 · 5 comments

Comments

@srl100
Copy link

srl100 commented Apr 4, 2024

I have a method defined in .fidl that works OK, taking a UInt32 input parameter and returning a 4 byte data structure, e.g.

SOME/IP Protocol (Service ID: 0x433f, Method ID: 0x0103, Length: 12)
    Service ID: 0x433f
    Method ID: 0x0103
    Length: 12
    Client ID: 0x0000
    Session ID: 0x0001
    SOME/IP Version: 0x01
    Interface Version: 0x01
    Message Type: 0x80 (Response)
    Return Code: 0x00 (Ok)
    Payload: 00000000

I now want to do extend my code to do some validity checking on the input parameter and insert a non-zero return code into the SOME/IP reply header if the checking fails.

I have naiively added an error clause to my .fidl and updated my implementation to fill in the error code, but all that seems to do is to insert my error code byte into the start of the data, i.e. returning 5 bytes of user data instead of 4 bytes.

SOME/IP Protocol (Service ID: 0x433f, Method ID: 0x0103, Length: 13)
    Service ID: 0x433f
    Method ID: 0x0103
    Length: 13        <--- Was 12
    Client ID: 0x0000
    Session ID: 0x0001
    SOME/IP Version: 0x01
    Interface Version: 0x01
    Message Type: 0x80 (Response)
    Return Code: 0x00 (Ok)   <--- Not what I want
    Payload: 3000000000

Note the change to the Length field and the extra "30" byte at the start of the data.

Here is my method specification with the new error section:

package commonapi.examples

interface Test {

  version {major 1 minor 0}

  enumeration eErrorCode {
    E_NO_ERROR=0x00                     /* No error */
    E_INVALID_INDEX=0x30                /* The requested index is out of range. */
  }

  struct sData {
    UInt32          Data1
  }

  method getData {
    in {
        UInt32 dataIndex
    }
    out {
        sData data
    }
    error eErrorCode
  }
}

My stub implementation is as follows:

#include "TestStubImpl.hpp"
#include <v1/commonapi/examples/Test.hpp>

using namespace v1::commonapi::examples;

TestStubImpl::TestStubImpl()
{
}

TestStubImpl::~TestStubImpl()
{
}

void TestStubImpl::getData(const std::shared_ptr<CommonAPI::ClientId> _client,
                                                uint32_t _dataIndex,
                                                getDataReply_t _reply)
{
    (void)_client;
    Test::eErrorCode error = Test::eErrorCode::E_INVALID_INDEX;
    Test::sData data = {};

    if (_dataIndex == 0UL)
    {
        error = Test::eErrorCode::E_NO_ERROR;
    }
    _reply(error, data);
}

In CommonAPI-SOMEIP_deployment_spec.fdepl I can see that SomeIpErrorCoding defaults to 'Header', but there does not appear to be another option - I don't know if that is relevant to this issue.

It looks to me like either I am missing a call into the SOME/IP stack to tell it that there has been an error, or alternatively that I am missing some configuration that tells the SOME/IP stack that the stub implementation of this method is providing the return code.

[Edited to display complete .fidl file, to show trace outputs straight from WireShark and to include stub implementation.]

Any ideas?

@rashen
Copy link

rashen commented Sep 4, 2024

I did some digging in this, and it looks like org.genivi.commonapi.someip/src/org/genivi/commonapi/someip/generator/FrancaSomeIPGeneratorExtensions.xtend has several places where it treats errors and out-arguments as a tuple, but I would expect it to be a variant instead. I just want to put a disclaimer here that I am not very familiar with the code base, or fluent with xtend, but I have the same problem.

@srl100
Copy link
Author

srl100 commented Sep 5, 2024

I get the impression from discussions such as here COVESA/vsomeip#712 that the developers consider there to be a strict boundary between the transport protocol (in this case SOME/IP) and the stub implementation, which stops the stub from seeing or changing anything in the transport protocol or below. I hope I am wrong.

@rashen
Copy link

rashen commented Sep 5, 2024

Yeah, I get the same impression. In the SomeIP speciifcation (R22-11, section 4.2.6.2) it say that having different layout depending on the error flag is recommended, but this generator is not following that recommendation. And since we can't see the error code from the stub you need to put everything in the payload instead.

@srl100
Copy link
Author

srl100 commented Sep 5, 2024

I'd say the requirement was stonger than a recommendation... R22-11 section 4.2.6.1 says that error codes 0x20..0x5E are "Reserved for specific errors of services and methods." which, to me, implies that a server's method implementation must be able to set the error code.

Putting "everything in the payload" is OK for home-brewed services where one has control of the specification, but my use case is to implement a VCIC (ISO 17215) server, which requires that methods check their input parameters and change the SOME/IP return code if the parameters are not acceptable.

@rashen
Copy link

rashen commented Sep 5, 2024

You are completely right, I missed that part.

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

No branches or pull requests

2 participants