Coder Social home page Coder Social logo

Comments (3)

abrauchli avatar abrauchli commented on July 20, 2024

Hi @Yousaf93,

Great to see an implementation. I quickly glanced over it and you're right that the timeout variant is better in this case, however I do have a few suggestions:

  • running into a timeout could (and typically will) mean that the read was successful (with less than max_data_len bytes), so I'd suggest always returning the actual number of bytes if there's been a write to data.
  • Make the timeout a #define - one second seems rather high given that we'll likely run into it most of the reads.
  • I'd wrap the printf(...) in a #ifdef DEBUG to avoid interfering with user output from the users of the library but I'll let the reviewers take their stance on that.
  • Lastly, it'd greatly help the reviewers if you could you resubmit the whole implementation (not just the rx function) in a folder in https://github.com/Sensirion/embedded-uart-sps/tree/master/embedded-uart-common/sample-implementations in a new pull request rather than copying it into this issue and apply proper indentation and formatting (I think the project has a .clang-format you can use to apply the project style automatically - there are quite a few whitespace errors which make the code hard to read even when indented). It'd also help if you wrote how exactly you tested your implementation (board you mentioned, sensor (SPS30?), connection setup to the board (if there's more than one UART port), what worked, what didn't and what wasn't tested).

Merry Christmas,
Andy

from embedded-uart-sps.

Yousaf93 avatar Yousaf93 commented on July 20, 2024

Hi @abrauchli , Thank you for your guidance
I would have definitely opted to the method mentioned but unfortunately i have strict timeline regarding the work so i did the work directly. But i would share the outcomes of the current implementations and would try to share the implementations later.
The current functions, Consider the Simple UART configurations and Poll for Serial data after transmit ,
As the SPS30 works on Master Slave MOSI MISO technique it responds as soon receives a command and with polling the reponse is missed which yields to timout error
So modificaction in following function is needed in place of HAL_UART_RX , The lenght data should be passed form the ISR of UART which needs to be initilaized in main
int16_t sensirion_shdlc_rx(uint8_t max_data_len,
struct sensirion_shdlc_rx_header* rxh,
uint8_t* data) {
int16_t len;
uint16_t i;
uint8_t rx_frame[SHDLC_FRAME_MAX_RX_FRAME_SIZE];
uint8_t* rx_header = (uint8_t*)rxh;
uint8_t j;
uint8_t crc;
uint8_t unstuff_next;

len = sensirion_uart_rx(2 + (5 + (uint16_t)max_data_len) * 2, rx_frame);
//printf("Total %d bytes received \n", len);

if (len < 1 || rx_frame[0] != SHDLC_START)
        return SENSIRION_SHDLC_ERR_MISSING_START;

    for (unstuff_next = 0, i = 1, j = 0; j < sizeof(*rxh) && i < len - 2; ++i) {
        if (unstuff_next) {
            rx_header[j++] = sensirion_shdlc_unstuff_byte(rx_frame[i]);
            unstuff_next = 0;
        } else {
            unstuff_next = sensirion_shdlc_check_unstuff(rx_frame[i]);
            if (!unstuff_next)
                rx_header[j++] = rx_frame[i];
        }
    }
    if (j != sizeof(*rxh) || unstuff_next)
        return SENSIRION_SHDLC_ERR_ENCODING_ERROR;

    if (max_data_len < rxh->data_len)
        return SENSIRION_SHDLC_ERR_FRAME_TOO_LONG; /* more data than expected */

    for (unstuff_next = 0, j = 0; j < rxh->data_len && i < len - 2; ++i) {
        if (unstuff_next) {
            data[j++] = sensirion_shdlc_unstuff_byte(rx_frame[i]);
            unstuff_next = 0;
        } else {
            unstuff_next = sensirion_shdlc_check_unstuff(rx_frame[i]);
            if (!unstuff_next)
                data[j++] = rx_frame[i];
        }
    }

    if (unstuff_next)
        return SENSIRION_SHDLC_ERR_ENCODING_ERROR;

    if (j < rxh->data_len)
        return SENSIRION_SHDLC_ERR_ENCODING_ERROR;

    crc = rx_frame[i++];
    if (sensirion_shdlc_check_unstuff(crc))
        crc = sensirion_shdlc_unstuff_byte(rx_frame[i++]);

    if (sensirion_shdlc_crc(rxh->addr + rxh->cmd + rxh->state, rxh->data_len,
                            data) != crc)
        return SENSIRION_SHDLC_ERR_CRC_MISMATCH;

    if (i >= len || rx_frame[i] != SHDLC_STOP)
        return SENSIRION_SHDLC_ERR_MISSING_STOP;

    return 0;

}

from embedded-uart-sps.

LeonieFierz avatar LeonieFierz commented on July 20, 2024

@Yousaf93 thank you for sharing the code. If you find time to submit the whole implementation as mentioned by abrauchli please open a new pull request.

from embedded-uart-sps.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.