Conversation
src/modules/module-avb/gptp.c
Outdated
| } | ||
|
|
||
| uint8_t buf[sizeof(struct ptp_management_msg) + sizeof(struct ptp_parent_data_set)]; | ||
| if (read(gptp->ptp_fd, &buf, sizeof(buf)) == -1) { |
There was a problem hiding this comment.
You should verify that the whole size was actually read:
There was a problem hiding this comment.
It fails now if the response is incomplete. Maybe I should wait for the rest in some way? Not sure how to do that in a nice way.
There was a problem hiding this comment.
Could you check if my change to read the buffer until it's complete is correct?
src/modules/module-avb/gptp.c
Outdated
|
|
||
| ioctl(gptp->ptp_fd, FIONREAD, &avail); | ||
| pw_log_debug("Flushing stale data: %u bytes", avail); | ||
| while (avail-- && read(gptp->ptp_fd, &tmp, 1)); |
There was a problem hiding this comment.
what happens if read return -1?
src/modules/module-avb/gptp.c
Outdated
| req.management_message_length_be = htobe16(2); | ||
| req.management_id_be = htobe16(PTP_MGMT_ID_PARENT_DATA_SET); | ||
|
|
||
| if (write(gptp->ptp_fd, &req, sizeof(req)) == -1) { |
There was a problem hiding this comment.
You should verify that the whole data is written.
There was a problem hiding this comment.
In the event of a partial write, the
caller can make another write() call to transfer the remaining
bytes. The subsequent call will either transfer further bytes or
may result in an error (e.g., if the disk is now full).
Does this mean that I just repeat the same write() although I don't want to write the part that was already written? Is it magic?
src/modules/module-avb/gptp.c
Outdated
| int avail; | ||
| uint8_t tmp; | ||
|
|
||
| ioctl(gptp->ptp_fd, FIONREAD, &avail); |
There was a problem hiding this comment.
Please check return is valid
| if ((res = setup_socket(server)) < 0) | ||
| goto error_free; | ||
|
|
||
| server->gptp = avb_gptp_new(server); |
There was a problem hiding this comment.
I think we should check for whether the return value is NULL
| str = pw_properties_get(impl->props, "ptp.management-socket"); | ||
| gptp->ptp_mgmt_socket_path = str ? strdup(str) : NULL; | ||
|
|
||
| if(gptp->ptp_mgmt_socket_path) |
There was a problem hiding this comment.
I realize in this case write an error, and stop everything, as it is a crucial element to continue.
There was a problem hiding this comment.
True. In the rtp-sap module the management socket thing is optional. Should we require the usage of the management socket? The management socket is optional in the PTP standards AFAIK.
This reverts commit 31babfd.
2 participants
This adds a new gptp object. It periodically checks the ptp management unix socket.
It should help us getting information of the ptp status, if it's locked, grandmaster changes, etc.
TODO: