[ncp-spi] improve callback implementation, change code style format #2734
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jwhui
reviewed
May 29, 2018
src/ncp/ncp_spi.hpp
Outdated
| { | ||
| kLength = 5, // SPI header length | ||
| kFlagReset = (1 << 7), | ||
| kFlagCrc = (1 << 6), |
There was a problem hiding this comment.
No. Not currently. I will remove it.
src/ncp/ncp_spi.hpp
Outdated
| static void SetFlagByte(uint8_t *aFrame, uint8_t aFlags) { aFrame[kIndexFlagByte] = aFlags; } | ||
| static uint8_t GetFlagByte(const uint8_t *aFrame) { return aFrame[kIndexFlagByte]; } | ||
|
|
||
| static bool IsFlagValid(const uint8_t *aFrame) |
There was a problem hiding this comment.
All these methods take a "frame". How about having a frame class that has non-static methods?
|
In new pushed commit, added |
pvanhorn
reviewed
May 30, 2018
src/ncp/ncp_spi.cpp
Outdated
| { | ||
| uint16_t rxDataLen = inputFrame.GetHeaderDataLen(); | ||
|
|
||
| if ((rxDataLen > 0) && (rxDataLen <= transDataLen) && (rxDataLen <= outputFrame.GetHeaderAcceptLen())) |
There was a problem hiding this comment.
This test is a little cryptic. Could you explain it with a comment.
pvanhorn
approved these changes
May 30, 2018
This commit makes the following changes in `NcpSpi` class: - It adds a new class `SpiFrame` which provides helper methods to parse and update the header fields in the header of an SPI frame. - It simplifies/enhances the `SpiTransactionComplete()` callback implementation by combining the parsing of input/output frame headers with the rx/tx frame processing. - It adds a check for correct pattern bits in a received frame flag byte (frames with incorrect pattern bits are ignored). - Code format is changed to follow the "pretty" style. - Fixes a rare issue with a possible incorrect accept length on the first SPI transaction after NCP reset.
|
Verified/tested this change on a device with SPI interface (to wpantund). |
jwhui
approved these changes
Jun 11, 2018
zhanglongxia
pushed a commit
to zhanglongxia/openthread
that referenced
this pull request
Nov 11, 2019
…s/COM-4614 to master * commit '769b0a5e74602c75600bfc218cbcff8a969bd164': (38 commits) [efr32] add BRD4304A (MGM12P) support and board-specific hal-config.h (openthread#2763) [mle] enhance the route table log message (openthread#2780) [posix] skip calling RadioReceiveDone with OT_ERROR_ABORT (openthread#2779) [docker] dockerfiles for openthread sim and wpantund (openthread#2760) [lowpan] use BigEndian::ReadUint16() (openthread#2774) [docs] updating Doxygen tags for ble module (openthread#2777) [toranj] add test-case for child table and child recovery (openthread#2776) [mesh-forwarder] do not log dropping of received beacon frames (openthread#2773) [string] adding ot::String<size> class (openthread#2764) [ncp-spi] improve callback implementation, change code style format (openthread#2734) [network-diagnostic] REED should omit ChildTable TLV request (openthread#2772) [data-poll] only allow data polling when rx-off-when-idle (openthread#2770) [mle] implement MLE Data Request retransmissions (openthread#2766) [mesh-forwarder] do not treat CCA failures the same as no-ack (openthread#2765) [nrf52840] fix cc310 with mbedtls 2.9.0 (openthread#2756) [mbedtls] do not treat visual studio build warnings as errors (openthread#2756) [mbedtls] remove v2.6.1 (openthread#2756) [scan-build] define KEY_EXCHANGE_ECDHE_ECDSA_ENABLED to avoid warnings (openthread#2756) [mbedtls] update cc2650 sha256_alt to new mbedtls API (openthread#2756) [mbedtls] update nrf52840 sha256_alt to new mbedtls APIs (openthread#2756) ...
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This commit makes the following changes in
NcpSpiclass:NcpSpi::Headerwhich provide helpermethods to parse and update the fields in the header of an SPI
frame.
SpiTransactionComplete()callbackimplementation by combining the parsing of input/output frame
headers with the rx/tx frame processing.
flag byte (frames with incorrect pattern bits are ignored).