Bump to 1.4.5 with latest note-c. (#105)

* remove note-c before re-add

* Squashed 'src/note-c/' content from commit e430f46

git-subtree-dir: src/note-c
git-subtree-split: e430f466219f0a34c2d4578c1fc717c26f5d9300

* Bump version.

library.properties

@@ -1,5 +1,5 @@
 name=Blues Wireless Notecard
-version=1.4.4
+version=1.4.5
 author=Blues Wireless
 maintainer=Blues Wireless <[email protected]>
 sentence=An easy to use Notecard Library for Arduino.

src/note-c/.devcontainer/devcontainer.json

@@ -7,8 +7,8 @@
     "context": "..",
 
     // Update the 'dockerFile' property if you aren't using the standard 'Dockerfile' filename.
-    //"dockerFile": "../.github/actions/compile-examples/Dockerfile",
-    "dockerFile": "../.github/actions/run_unit_tests/Dockerfile",
+    "image": "ghcr.io/blues/note_c_ci:latest",
+    // "dockerFile": "../Dockerfile",
 
     // Set *default* container specific settings.json values on container create.
     "settings": {},

src/note-c/.github/workflows/ci.yml

@@ -7,28 +7,100 @@ on:
     branches: [ master ]
 
 jobs:
+  check_dockerfile_changed:
+    runs-on: ubuntu-latest
+    outputs:
+      changed: ${{ steps.filter.outputs.changed }}
+
+    steps:
+      - name: Checkout code
+        uses: actions/checkout@v3
+
+      # TODO: This is a 3rd party GitHub action from some dude. Ideally, we'd
+      #       use something more "official".
+      - name: Check if Dockerfile changed
+        uses: dorny/paths-filter@v2
+        id: filter
+        with:
+          base: 'master'
+          filters: |
+            changed:
+              - 'Dockerfile'
+
   run_unit_tests:
     runs-on: ubuntu-latest
+    permissions:
+      contents: read
+      packages: write
+    needs: check_dockerfile_changed
+
     steps:
-      - name: Checkout Code
+      - name: Checkout code
         uses: actions/checkout@v3
-      - name: Run unit tests
-        uses: ./.github/actions/run_unit_tests
-      - name: Coveralls Action Bug Workaround
-        id: coveralls_bug_workaround
-        run: sudo sed -i 's/github\/workspace/home\/runner\/work\/note-c\/note-c/g' ./build/test/coverage/lcov.info
-      - name: Publish Test Coverage
-        id: publish_coverage
+
+      - name: Login to GitHub Container Registry
+        uses: docker/login-action@v2
+        with:
+          registry: ghcr.io
+          username: ${{ github.actor }}
+          password: ${{ secrets.GITHUB_TOKEN }}
+
+      - name: Set up Docker Buildx
+        uses: docker/setup-buildx-action@v2
+
+      - name: Rebuild image if Dockerfile changed
+        if: ${{ needs.check_dockerfile_changed.outputs.changed == 'true' }}
+        uses: docker/build-push-action@v4
+        with:
+          load: true
+          tags: ghcr.io/blues/note_c_ci:latest
+
+      - name: Run tests
+        run: |
+          docker run --rm --volume $(pwd):/note-c/ --workdir /note-c/ --entrypoint ./scripts/run_unit_tests.sh ghcr.io/blues/note_c_ci:latest --coverage --mem-check
+
+      - name: Adjust lcov source file paths for Coveralls
+        run: sudo sed -i 's/\/note-c\///g' ./build/test/coverage/lcov.info
+
+      - name: Publish test coverage
         uses: coverallsapp/github-action@master
         with:
           github-token: ${{ secrets.GITHUB_TOKEN }}
           path-to-lcov: ./build/test/coverage/lcov.info
+
   run_astyle:
     runs-on: ubuntu-latest
+
     steps:
       - name: Checkout Code
         uses: actions/checkout@v3
+
       - name: Install astyle
         run: sudo apt-get install -y astyle
+
       - name: Check formatting
         run: ./scripts/run_astyle.sh
+
+  publish_ci_image:
+    runs-on: ubuntu-latest
+    # Make sure unit tests unit tests passed before publishing.
+    needs: [check_dockerfile_changed, run_unit_tests]
+    # Only publish the image if this is a push event and Dockerfile changed.
+    if: ${{ github.event_name == 'push' && needs.check_dockerfile_changed.outputs.changed == 'true' }}
+
+    steps:
+      - name: Login to GitHub Container Registry
+        uses: docker/login-action@v2
+        with:
+          registry: ghcr.io
+          username: ${{ github.actor }}
+          password: ${{ secrets.GITHUB_TOKEN }}
+
+      - name: Set up Docker Buildx
+        uses: docker/setup-buildx-action@v2
+
+      - name: Push image to registry
+        uses: docker/build-push-action@v4
+        with:
+          push: true
+          tags: ghcr.io/blues/note_c_ci:latest

src/note-c/.github/actions/run_unit_tests/Dockerfile → src/note-c/Dockerfile

@@ -75,5 +75,3 @@ RUN ["dash", "-c", "\
  && cmake --build build/ --target install \
  && rm -rf Catch2-3.2.1  v3.2.1.tar.gz \
 "]
-
-ENTRYPOINT ["./scripts/run_unit_tests.sh", "--coverage", "--mem-check"]

src/note-c/n_request.c

@@ -35,6 +35,7 @@ static uint16_t lastRequestSeqno = 0;
 NOTE_C_STATIC int32_t crc32(const void* data, size_t length);
 NOTE_C_STATIC char *crcAdd(char *json, uint16_t seqno);
 NOTE_C_STATIC bool crcError(char *json, uint16_t shouldBeSeqno);
+static bool notecardSupportsCrc = false;
 #endif
 
 /**************************************************************************/
@@ -425,23 +426,22 @@ J *NoteTransaction(J *req)
             _Free(responseJSON);
             errStr = "crc error {io}";
             lastRequestRetries++;
-            _Debugln("retrying CRC error on notecard response detected by host");
+            _Debugln("CRC error on response");
             _DelayMs(500);
             continue;
         }
 
-        // There's a possibility that we got back a response that has an I/O
-        // error.  In order to determine this, we'll need to unmarshal it.  As
-        // such, do this first by brute force and then 'correctly'.
-        if (strstr((char *)responseJSON, c_ioerr) == NULL) {
-            break;
-        }
+        // See if the response JSON can't be unmarshaled, or if it contains an {io} error
         J *rsp = JParse(responseJSON);
-        if (rsp == NULL) {
-            break;
+        bool isIoError = (rsp == NULL);
+        if (rsp != NULL) {
+            isIoError = NoteErrorContains(JGetString(rsp, c_err), c_ioerr);
+            if (isIoError) {
+                _Debug("err: ");
+                _Debugln(JGetString(rsp, c_err));
+            }
+            JDelete(rsp);
         }
-        bool isIoError = NoteErrorContains(JGetString(rsp, c_err), c_ioerr);
-        JDelete(rsp);
         if (isIoError) {
             _Free(responseJSON);
             errStr = "notecard i/o error {io}";
@@ -734,10 +734,14 @@ NOTE_C_STATIC bool crcError(char *json, uint16_t shouldBeSeqno)
     if (jsonLen < CRC_FIELD_LENGTH+2 || json[jsonLen-1] != '}') {
         return false;
     }
+    // See if it has a compliant CRC field
     size_t fieldOffset = ((jsonLen-1) - CRC_FIELD_LENGTH);
     if (memcmp(&json[fieldOffset+CRC_FIELD_NAME_OFFSET], CRC_FIELD_NAME_TEST, sizeof(CRC_FIELD_NAME_TEST)-1) != 0) {
-        return false;
+        // If we've seen a CRC before, we should see one every time
+        return notecardSupportsCrc ? true : false;
     }
+    // If we get here, we've seen at least one CRC from the Notecard, so we should expect it.
+    notecardSupportsCrc = true;
     char *p = &json[fieldOffset + CRC_FIELD_NAME_OFFSET + (sizeof(CRC_FIELD_NAME_TEST)-1)];
     uint16_t actualSeqno = (uint16_t) n_atoh(p, 4);
     uint32_t actualCrc32 = (uint32_t) n_atoh(p+5, 8);

src/note-c/test/src/NoteTransaction_test.cpp

@@ -225,7 +225,7 @@ TEST_CASE("NoteTransaction")
 
         J *resp = NoteTransaction(req);
 
-        CHECK(NoteJSONTransaction_fake.call_count == 1);
+        CHECK(NoteJSONTransaction_fake.call_count >= 1);
         CHECK(resp != NULL);
         // Ensure there's an error in the response.
         CHECK(NoteResponseError(resp));

src/note-c/test/src/crcError_test.cpp

@@ -73,6 +73,13 @@ TEST_CASE("crcError")
 
             NoteFree(jsonWithCrc);
         }
+
+        SECTION("Trailing CRLF") {
+            char json[] = "{\"req\":\"hub.sync\",\"crc\":\"0001:10BAC79A\"}\r\n";
+
+            // Trailing \r\n should be ignored.
+            CHECK(!crcError(json, seqNo));
+        }
     }
 }