Feature/support multiple clients

[Hadatko]

Pull request

Choose Correct

• bug
• feature

Describe the pull request

Better support of C++ usecases -> mainly multiple clients usage.

To Reproduce

Expected behavior

test_passing :D

Screenshots

Desktop (please complete the following information):

• OS:
• eRPC Version:

Steps you didn't forgot to do

• [X I checked if other PR isn't solving this issue.
• I read Contribution details and did appropriate actions.
• PR code is tested.
• PR code is formatted.

Additional context

[Hadatko]

Currently WIP (alpha)

[Hadatko]

@MichalPrincNXP test_annotation can be compiled. I moved C related generated code into its own files. This way C++ code will be cleaner. Also more transparent for C develepors how the C++ code is wrapped.

[mnathieu]

Hi!
I am really interested in the feature (especially the C++ version). Great addition.
Are you still active on it? Any plan to add it to a future release?

[Hadatko]

HI @mnathieu , it is still my top priority. I had solution twice, but twice it was not complete due to issues with callbacks and c++. I am expecting that i will finish integration to the end of this year. Question is if documentation update will be part of it, or i will add documentation update next year. I guess release depend on that.

[Hadatko]

HI @MichalPrincNXP,
FYI: tests are passing. Errors which appear are due to pytest which need to be modified and some clang compiler incompatibility.
I need refactor code a bit , but that shouldn't be an huge change.

[tmordeko]

Hi @Hadatko we are looking forward for this feature , when do you plan to release it ?

[Hadatko]

Hi @tmordeko , i think i will be able to release it here (to develop branch) before March 2023. It looks working well now, but code need be edited a bit (formatted/refactored). Maybe release it in small steps (i already created few smaller PRs).
Oficial release to master should be around June/July.

[Hadatko]

Hi @MichalPrincNXP could you take o look on this code? It should be ready more or less. If you would like it i would improve pytests and update documentation.

Hi @amgross i am curious about tour opinion too. Do you think you could review it as well?

One thing which i know is not good is generating common datatypes. I wanted to have two header files. One for c and one for c++. For c++ i wanted to hide them under namescope erpcShim. I had compilation issues which i think i would be able to solve (related const definition). But then i started to think if it is good idea and if i should rather use global namespace.

[Hadatko]

@MichalPrincNXP i made changes based on your PR. Let me know if they met your requests. Also i enabled in clang config to keep empty lines between includes, which will simplify keeping includes on some specific position...

[Hadatko]

@MichalPrincNXP @PhilippHaefele for know i want finish this PR and we don't have much time so i reverted commit with enum classes support in shim code. And we can create new PR where we will solve that.

[MichalPrincNXP]

@MichalPrincNXP i made changes based on your PR. Let me know if they met your requests. Also i enabled in clang config to keep empty lines between includes, which will simplify keeping includes on some specific position...

Thank you, let me check it again on hw

[MichalPrincNXP]

Hi Dusan,
I have successfully converted the NXP erpc_matrix_multipy example to use the latest generator and the erpc code. However, I am facing issues with the erpc_two_way_rpc example.
First, incorrect includes are generated in several shim code files:
#include "erpc_two_way_rpc_Core0Interface.h" -> #include "erpc_two_way_rpc_Core0Interface**_common**.h"
Second, incorrect generated code in erpc_two_way_rpc_Core0Interface_server.cpp / Core0Interface_interface_getNumberCallback_t_shim function:

        if ((serviceID == ) && (functionID == 4))
        {
            m_handler->getNumberFromCore1(param1);
        }

Error[Pe029]: expected an expression	C:\D\work\sdk_w\mcu-sdk-2.0\middleware\multicore\example\multicore_examples\erpc_common\erpc_two_way_rpc\service\erpc_two_way_rpc_Core0Interface_server.cpp	53	
Error[Pe167]: argument of type "uint32_t" is incompatible with parameter of type "uint32_t *"	C:\D\work\sdk_w\mcu-sdk-2.0\middleware\multicore\example\multicore_examples\erpc_common\erpc_two_way_rpc\service\erpc_two_way_rpc_Core0Interface_server.cpp	55	

Could you please help to address these issues?

[Hadatko]

Hi @MichalPrincNXP , thank you. I will take a look on these. I didn't expect issue as our tests are passing. Maybe we need updated tests...

[Hadatko]

@MichalPrincNXP Did you use newly compiled erpcgen from this branch (you need do it by yourself or download from CI). DId you update erpc file (current one is no longer supported. You can see in tests/test_callback).

[MichalPrincNXP]

Yes, the newly compiled erpcgen has been used and the erpc file updated. Could you please review attached updated erpc first? Hope I did it correctly.
erpc_two_way_rpc.zip

[Hadatko]

So i took a look into your erpc file. Callback was defined as global now moved into two interfaces. This mean that the callback will be created twice (one for each interface) and it will be used only within interface where it is declared. If you take look into test_callback.erpc changes, the callback is created only in one interface and in second interface it is referenced trough "Interface::". This is enabling to us define from which interface callback should be used.

[MichalPrincNXP]

I see, I am trying to use Core1Interface:: / Core0Interface:: but still facing issues ... may I ask you if you could provide the updated erpc_two_way_rpc.erpc?
Still, I afraid includes in shim code are not accurate (missing _common postfixes)
Thanks

[Hadatko]

@MichalPrincNXP I noticed that erpc file is using includes to non existing files. I woudl remove them. Anyway it looks like there is still an issue using callback type in return function. So i will investigate this part
erpc_two_way_rpc.erpc.zip

[Hadatko]

hi @MichalPrincNXP , i am struggling a bit with compiling provided example (i will try to do it). In a mean time i think i fixed both observated errors. Could you try newer erpcgen based on latest commit?

[MichalPrincNXP]

hi @MichalPrincNXP , i am struggling a bit with compiling provided example (i will try to do it). In a mean time i think i fixed both observated errors. Could you try newer erpcgen based on latest commit?

sure

[MichalPrincNXP]

Hi Dusan, still one build issue in erpc_two_way_rpc_Core0Interface_server.cpp/erpc_two_way_rpc_Core1Interface_server.cpp shim code:

erpc_status_t Core1Interface_service::getNumberFromCore0_shim(Codec * codec, MessageBufferFactory *messageFactory, uint32_t sequence)
{
    erpc_status_t err = kErpcStatus_Success;

    uint32_t number;

    // startReadMessage() was already called before this shim was invoked.

    err = codec->getStatus();
    if (err == kErpcStatus_Success)
    {
        // Invoke the actual served function.
#if ERPC_NESTED_CALLS_DETECTION
        nestingDetection = true;
#endif
        m_handler->getNumberFromCore0(&param1);
#if ERPC_NESTED_CALLS_DETECTION
        nestingDetection = false;
#endif

Error[Pe020]: identifier "param1" is undefined

[Hadatko]

So there is bug i should repair quickly. It can be ignored if definition is changed to: getNumberCallback2_t getNumberFromCore0;
But i will push fix one ready.

[Hadatko]

@MichalPrincNXP Fix was easy (actually modifying previous commit). I could do it before but i thought that i can break something. If i would do it we wouldn't have this error :DDD So i took a look on changes with this fix and it really changed only one line in generated shim code so i believe if this was the only error, that it will work properly.

[Hadatko]

@MichalPrincNXP SO it broke something as i was affraid, but now fix everywhere ;) Still modifying previous config only...

[MichalPrincNXP]

Thanks, build is ok now. I have to adjust the erpc_two_way example logic a little and once it is working again I could integrate this PR.

[MichalPrincNXP]

Finished, thank you Dusan for this new erpc feature.

[Hadatko]

;) Thank you for testing and finding bugs