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.

Sign up for GitHub

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

Jump to bottom

Fix misra erpc c #158

Merged
merged 14 commits into from
Feb 17, 2021
Merged

Fix misra erpc c #158

merged 14 commits into from
Feb 17, 2021

Conversation

Hadatko
Copy link
Member

@Hadatko Hadatko commented Jan 21, 2021

This is quite complex commit to fix Misra in erpc_c folder. Some files changed a logic a bit. Take your time with review.

@Hadatko Hadatko self-assigned this Jan 21, 2021
@MichalPrincNXP
Copy link
Member

Hello Dusan , I am sorry for late response, please give me some time for the review, I would like to have a look at it next week.

@MichalPrincNXP MichalPrincNXP self-requested a review February 10, 2021 12:41
MichalPrincNXP
MichalPrincNXP requested changes Feb 10, 2021
View reviewed changes
Copy link
Member

@MichalPrincNXP MichalPrincNXP left a comment

Choose a reason for hiding this comment

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

acceptance tests are ok, I have also triggered build of examples + unit test on boards
could you please address my 4 comments?
btw, lot's of work has been done in this PR, thanks for this big effort!

erpc_c/port/erpc_sysgpio.h Outdated Show resolved Hide resolved
erpc_c/setup/erpc_setup_usb_cdc.cpp Outdated Show resolved Hide resolved
erpc_c/transports/erpc_spi_slave_transport.cpp Outdated Show resolved Hide resolved
erpc_c/transports/erpc_uart_cmsis_transport.h Outdated Show resolved Hide resolved
@Hadatko
Copy link
Member Author

Hadatko commented Feb 10, 2021

@MichalPrincNXP Good catch. Good eyes. PR fix pushed.

MichalPrincNXP
MichalPrincNXP reviewed Feb 11, 2021
View reviewed changes
erpc_c/transports/erpc_mu_transport.cpp Outdated Show resolved Hide resolved
erpc_c/infra/erpc_common.h Show resolved Hide resolved
erpc_c/transports/erpc_dspi_slave_transport.cpp Outdated Show resolved Hide resolved
erpc_c/transports/erpc_spi_slave_transport.cpp Show resolved Hide resolved
erpc_c/transports/erpc_spi_slave_transport.cpp Outdated Show resolved Hide resolved
@MichalPrincNXP
Copy link
Member

Hi Dusan, I think all seems to be OK now. What I am missing yet is the erpc_dspi_slave_transport.cpp update to be aligned with changes in erpc_spi_slave_transport.cpp (underlyingSend method, removing ifndef, adding else). Could you please review attached proposal and add it into the PR if you are OK? Thanks.
erpc_dspi_slave_transport.zip

@Hadatko
Copy link
Member Author

Hadatko commented Feb 17, 2021

Hi Dusan, I think all seems to be OK now. What I am missing yet is the erpc_dspi_slave_transport.cpp update to be aligned with changes in erpc_spi_slave_transport.cpp (underlyingSend method, removing ifndef, adding else). Could you please review attached proposal and add it into the PR if you are OK? Thanks.
erpc_dspi_slave_transport.zip

I thought u pushed that. Sorry. Should be fine now.

@MichalPrincNXP MichalPrincNXP merged commit 175d889 into EmbeddedRPC:develop Feb 17, 2021
@MichalPrincNXP
Copy link
Member

Thank you Dusan for this extensive PR and the code improvement!

@Hadatko Hadatko deleted the fixMisraErpcC branch February 23, 2021 13:06
@Hadatko
Copy link
Member Author

Hadatko commented Feb 23, 2021

I missed this merge. Thank you for deep review and merge.

MichalPrincNXP added a commit that referenced this pull request Jul 16, 2021
@MichalPrincNXP
eRPC updates 07/2021
9849c5d 
-- Fix the order of commands in underlyingReceive and underlyingSend of UART CMSIS transport.
-- Fix (revert) the sequence of commands in MUTransport::rx_cb (bug unintentionally introduced by Github pull request #158).
-- Identify the used LINK hw during the runtime (LPCLINK2 vs. MCULINK) in transport.py.
-- Improve I2cSlaveTransport
-- Add reference links into erpc/README.md.
-- Update license file.
@MichalPrincNXP MichalPrincNXP mentioned this pull request Nov 16, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@MichalPrincNXP MichalPrincNXP MichalPrincNXP approved these changes

Assignees

@Hadatko Hadatko

Labels
enhancement
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Hadatko @MichalPrincNXP