freifunk-darmstadt / quectel-timesync

GNU General Public License v2.0
0 stars 1 forks source link

`validate_response` implementation is problematic? #1

Open szescxz opened 7 months ago

szescxz commented 7 months ago

Tested with commit 4333888cb8025b92511597a95859943fae0a0bc8 on aarch64, applied a small patch to see what's going on:

--- a/quectel-timesync.c
+++ b/quectel-timesync.c
@@ -117,3 +117,4 @@
    if (strlen("\"2023/10/07,23:07:16+08,1\"") != response_len) {
+       if (debug) fprintf(stdout, "example = %ld, actual = %d\n", strlen("\"2023/10/07,23:07:16+08,1\""), response_len);
        return -1;
    }

Result:

# quectel-timesync -v -p /dev/ttyUSB3
example = 26, actual = 27
Invalid response: "2023/11/16,07:05:34+32,0"
szescxz commented 7 months ago

After digging further I realized my modem is using CRLF while quectel-timesync only handles LF.

--- a/quectel-timesync.c
+++ b/quectel-timesync.c

@@ -105,6 +105,9 @@ int read_response(int fd, const char *response, char *buf, int buf_size)
                break;
            case READ_STATE_CONTENT:
                if (input_char == '\n') {
+                   if (*(ptr - 1) == '\r')
+                       ptr--;
+
                    *ptr = '\0';
                    return ptr - buf;
                } else {
blocktrron commented 7 months ago

@szescxz I'll look info this. Can you tell me which modem you are seeing this behavior on?

szescxz commented 7 months ago

RM520N-GL, works if applied the patch above. Although I'll need to ensure quectel-timesync has exclusive access to the serial port, but that would be another story.

blocktrron commented 7 months ago

I think we have the same modem around. I'll check.