-
Notifications
You must be signed in to change notification settings - Fork 245
This are some enhance feature for erpc multi-processor. #1
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
Conversation
Signed-off-by: Hake Huang <hake.huang@nxp.com>
Signed-off-by: Hake Huang <hake.huang@nxp.com>
Signed-off-by: Hake Huang <hake.huang@nxp.com>
|
Hi @hakehuang, thanks for the PR. Will review shortly. |
erpc_c/setup/erpc_server_setup.cpp
Outdated
| #include "simple_server.h" | ||
| #include "manually_constructed.h" | ||
| #include "basic_codec.h" | ||
| #include "message_buffer.h" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#include "message_buffer.h" and #include "erpc_config_internal.h" should be moved to erpc_setup.h
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right, updated at 142ecdb
Signed-off-by: Hake Huang <hake.huang@nxp.com>
| } | ||
| } | ||
| }; | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based on these changes this code "static ManuallyConstructed s_msgFactory;" and "
static ManuallyConstructed s_codecFactory;" from client/server setup files should be moved here too.
This header file #include "message_buffer.h" from client/server setup files should be removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
move the static variable from server and client to a common place will have problem when you application have both server and client functions. e.g. in my test application, I use a run time command to switch between client and server, which make the static variable remains for server/client is better.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK i didn't realized it. Is this switching client and server requested in your application? eRPC has appi for using client and server both in one application. It is covered by files containg word "arbitrator". Unfortunately we hadn't enough time for creating application with using this feature. But if you want i can send you more information about it. I am also working on documentation improvements where this feature is described.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, please.
Signed-off-by: Hake Huang <hake.huang@nxp.com>
Signed-off-by: Hake Huang <hake.huang@nxp.com>
| SPI_Type *m_spiBaseAddr; /*!< Base address of DSPI peripheral used in this transport layer */ | ||
| uint32_t m_baudRate; /*!< Baud rate of DSPI peripheral used in this transport layer */ | ||
| uint32_t m_srcClock_Hz; /*!< Source clock of DSPI peripheral used in this transport layer */ | ||
| bool m_binited; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right, commit 3174e60
Signed-off-by: Hake Huang <hake.huang@nxp.com>
Signed-off-by: Hake Huang <hake.huang@nxp.com>
This reverts commit 8c14710.
flit
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just a few minor changes requested to match coding style. Otherwise LGTM! Thanks for your PR.
Note that we'll probably be reworking the message buffer factory stuff for the setup API shortly. Probably will pass in a message buffer factory instance pointer similar to how the transport is passed in. The use case is for supporting the RPMsg-specific message buffer factories that enable zero-copy.
|
|
||
| DspiSlaveTransport::~DspiSlaveTransport() | ||
| { | ||
| if(m_binited) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add a space between the "if" and "(" to match our coding style (same coding style as KSDK).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK
| SPI_Type *m_spiBaseAddr; /*!< Base address of DSPI peripheral used in this transport layer */ | ||
| uint32_t m_baudRate; /*!< Baud rate of DSPI peripheral used in this transport layer */ | ||
| uint32_t m_srcClock_Hz; /*!< Source clock of DSPI peripheral used in this transport layer */ | ||
| bool m_binited; /*!< the SPI peripheral init status flag */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rename m_binited to m_isInited. We don't use type prefixes on names.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
erpc_c/setup/erpc_setup.h
Outdated
| using namespace erpc; | ||
|
|
||
| //////////////////////////////////////////////////////////////////////////////// | ||
| // CLASS |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change "CLASS" to mixed case "Classes".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok
|
|
||
| #ifndef _EMBEDDED_RPC_SETUP_H_ | ||
| #define _EMBEDDED_RPC_SETUP_H_ | ||
| #include <new> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Insert blank line between #define and first #include.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK
Signed-off-by: Hake Huang <hake.huang@nxp.com>
|
Thanks! LGTM. |
…length This was spotted on a SAML21 controller: #0 usart_sync_read (io_descr=0x200006dc <USART_0>, buf=0x20000378 <s_msgFactory+8> , length=0) at ../hal/src/hal_usart_sync.c:271 EmbeddedRPC#1 0x000001e0 in io_read (io_descr=0x200006dc <USART_0>, buf=0x20000378 <s_msgFactory+8> , length=0) at ../hal/src/hal_io.c:62 EmbeddedRPC#2 0x0000e3da in erpc::UsartSyncTransport::underlyingReceive (this=0x20000578 <s_transport>, data=0x20000378 <s_msgFactory+8> , size=0) at ../erpc_usart_sync_transport.cpp:29 EmbeddedRPC#3 0x0000dd96 in erpc::FramedTransport::receive (this=0x20000578 <s_transport>, message=0x200026c4) at ../erpc_framed_transport.cpp:63 EmbeddedRPC#4 0x0000d7da in erpc::SimpleServer::runInternalBegin (this=0x20000340 <s_server>, codec=0x200026c0, buff=..., msgType=@0x200026bf: 32, serviceId=@0x200026b8: 536880832, methodId=@0x200026b4: 536871784, sequence=@0x200026b0: 536871784) at ../erpc_simple_server.cpp:64 EmbeddedRPC#5 0x0000d72a in erpc::SimpleServer::runInternal (this=0x20000340 <s_server>) at ../erpc_simple_server.cpp:42 EmbeddedRPC#6 0x0000d99e in erpc::SimpleServer::poll (this=0x20000340 <s_server>) at ../erpc_simple_server.cpp:223 EmbeddedRPC#7 0x0000d44e in erpc_server_poll () at ../erpc_server_setup.cpp:97 EmbeddedRPC#8 0x00006fa4 in main () at ../main.c:72 The UART need to be restarted to recover from a such error, for ex.: if (erpc_server_poll()) { usart_sync_disable(&USART_0); usart_sync_enable(&USART_0); }
- add west, gcc-arm-none-eabi, mcux-sdk repo
the transport need rewrite and the porting folder files need to be take care