Skip to content

feat: add error display and read/send iteration#31

Open
t-nakagawa-esol wants to merge 1 commit intoautowarefoundation:mainfrom
t-nakagawa-esol:main
Open

feat: add error display and read/send iteration#31
t-nakagawa-esol wants to merge 1 commit intoautowarefoundation:mainfrom
t-nakagawa-esol:main

Conversation

@t-nakagawa-esol
Copy link

  1. The current implementation cannot distinguish between time-outs and other errors for the receive/send function.
    So, we might have to add a specified number of retries for the other errors below.
    EAGAIN , EWOULDBLOCK or EINTR for read/send

  2. It might not be possible to guarantee that sizeof(frame) will always return in the return value of the read/send function
    Therefore, it might be necessary to repeat until the size of the return value becomes the sizeof(frame).

  3. In the wait function, the current implementation does not distinguish between timeouts and other errors.
    Therefore, it might be necessary to display errno for errors other than timeout.

EAGAIN , EWOULDBLOCK or EINTR for read/send function.
2. Get read/send function repeat until the size of the return value becomes the sizeof(frame).
3. Display errno for errors other than timeout in wait function.
Comment on lines +158 to +160
nbytes = ::send(
m_file_descriptor, (char *)p + bytes_sent, sizeof(data_frame) - bytes_sent,
flags);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
nbytes = ::send(
m_file_descriptor, (char *)p + bytes_sent, sizeof(data_frame) - bytes_sent,
flags);
nbytes = ::send(
m_file_descriptor, reinterpret_cast<char *>(p) + bytes_sent, sizeof(data_frame) - bytes_sent,
flags);

Related test error: https://github.com/autowarefoundation/ros2_socketcan/actions/runs/9974311030/job/27561524880?pr=31#step:5:1406

@xmfcx xmfcx changed the title Added error display and read/send iteration feat: add error display and read/send iteration Dec 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants