From c2c06ed0adb3f779d88ab6f812a380d848c72c75 Mon Sep 17 00:00:00 2001 From: Eric Severance Date: Wed, 1 Jan 2025 16:40:14 -0800 Subject: [PATCH 01/11] Move DecodedServiceEnvelope into its own file (#5715) --- src/mqtt/MQTT.cpp | 19 +------------------ src/mqtt/ServiceEnvelope.cpp | 23 +++++++++++++++++++++++ src/mqtt/ServiceEnvelope.h | 13 +++++++++++++ 3 files changed, 37 insertions(+), 18 deletions(-) create mode 100644 src/mqtt/ServiceEnvelope.cpp create mode 100644 src/mqtt/ServiceEnvelope.h diff --git a/src/mqtt/MQTT.cpp b/src/mqtt/MQTT.cpp index 4260ae201..5141af560 100644 --- a/src/mqtt/MQTT.cpp +++ b/src/mqtt/MQTT.cpp @@ -2,6 +2,7 @@ #include "MeshService.h" #include "NodeDB.h" #include "PowerFSM.h" +#include "ServiceEnvelope.h" #include "configuration.h" #include "main.h" #include "mesh/Channels.h" @@ -25,7 +26,6 @@ #endif #include #include -#include #include #include @@ -47,23 +47,6 @@ static uint8_t bytes[meshtastic_MqttClientProxyMessage_size + 30]; // 12 for cha static bool isMqttServerAddressPrivate = false; -// meshtastic_ServiceEnvelope that automatically releases dynamically allocated memory when it goes out of scope. -struct DecodedServiceEnvelope : public meshtastic_ServiceEnvelope { - DecodedServiceEnvelope() = delete; - DecodedServiceEnvelope(const uint8_t *payload, size_t length) - : meshtastic_ServiceEnvelope(meshtastic_ServiceEnvelope_init_default), - validDecode(pb_decode_from_bytes(payload, length, &meshtastic_ServiceEnvelope_msg, this)) - { - } - ~DecodedServiceEnvelope() - { - if (validDecode) - pb_release(&meshtastic_ServiceEnvelope_msg, this); - } - // Clients must check that this is true before using. - const bool validDecode; -}; - inline void onReceiveProto(char *topic, byte *payload, size_t length) { const DecodedServiceEnvelope e(payload, length); diff --git a/src/mqtt/ServiceEnvelope.cpp b/src/mqtt/ServiceEnvelope.cpp new file mode 100644 index 000000000..ee55f22f6 --- /dev/null +++ b/src/mqtt/ServiceEnvelope.cpp @@ -0,0 +1,23 @@ +#include "ServiceEnvelope.h" +#include "mesh-pb-constants.h" +#include + +DecodedServiceEnvelope::DecodedServiceEnvelope(const uint8_t *payload, size_t length) + : meshtastic_ServiceEnvelope(meshtastic_ServiceEnvelope_init_default), + validDecode(pb_decode_from_bytes(payload, length, &meshtastic_ServiceEnvelope_msg, this)) +{ +} + +DecodedServiceEnvelope::DecodedServiceEnvelope(DecodedServiceEnvelope &&other) + : meshtastic_ServiceEnvelope(meshtastic_ServiceEnvelope_init_zero), validDecode(other.validDecode) +{ + std::swap(packet, other.packet); + std::swap(channel_id, other.channel_id); + std::swap(gateway_id, other.gateway_id); +} + +DecodedServiceEnvelope::~DecodedServiceEnvelope() +{ + if (validDecode) + pb_release(&meshtastic_ServiceEnvelope_msg, this); +} \ No newline at end of file diff --git a/src/mqtt/ServiceEnvelope.h b/src/mqtt/ServiceEnvelope.h new file mode 100644 index 000000000..6ab0695db --- /dev/null +++ b/src/mqtt/ServiceEnvelope.h @@ -0,0 +1,13 @@ +#pragma once + +#include "mesh/generated/meshtastic/mqtt.pb.h" + +// meshtastic_ServiceEnvelope that automatically releases dynamically allocated memory when it goes out of scope. +struct DecodedServiceEnvelope : public meshtastic_ServiceEnvelope { + DecodedServiceEnvelope(const uint8_t *payload, size_t length); + DecodedServiceEnvelope(DecodedServiceEnvelope &) = delete; + DecodedServiceEnvelope(DecodedServiceEnvelope &&); + ~DecodedServiceEnvelope(); + // Clients must check that this is true before using. + const bool validDecode; +}; \ No newline at end of file From 9f32995d7fcf96384dc01b5b953772327a600e97 Mon Sep 17 00:00:00 2001 From: Eric Severance Date: Wed, 1 Jan 2025 17:25:01 -0800 Subject: [PATCH 02/11] Implement MeshModule destructor (#5714) --- src/mesh/MeshModule.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/mesh/MeshModule.cpp b/src/mesh/MeshModule.cpp index 9de54ade5..2f2863fa5 100644 --- a/src/mesh/MeshModule.cpp +++ b/src/mesh/MeshModule.cpp @@ -4,6 +4,7 @@ #include "NodeDB.h" #include "configuration.h" #include "modules/RoutingModule.h" +#include #include std::vector *MeshModule::modules; @@ -29,7 +30,9 @@ void MeshModule::setup() {} MeshModule::~MeshModule() { - assert(0); // FIXME - remove from list of modules once someone needs this feature + auto it = std::find(modules->begin(), modules->end(), this); + assert(it != modules->end()); + modules->erase(it); } meshtastic_MeshPacket *MeshModule::allocAckNak(meshtastic_Routing_Error err, NodeNum to, PacketId idFrom, ChannelIndex chIndex, From 183f68ba0005e9bfeb4423681693b225d28a5f6d Mon Sep 17 00:00:00 2001 From: Eric Severance Date: Wed, 1 Jan 2025 17:26:12 -0800 Subject: [PATCH 03/11] Run tests as part of the main CI (#5712) * Create an shared action to install native dependecies * Create a workflow for running native tests * Artifact names contain version * Add test-native to main_matrix.yml * No permission are required for test_native.yml * Add permissions for dorny/test-reporter * No permissions when running tests * s/Generate Reports/Generate Test Reports/ --- .github/actions/setup-native/action.yml | 14 +++ .github/workflows/build_native.yml | 20 +--- .github/workflows/main_matrix.yml | 3 + .github/workflows/test_native.yml | 153 ++++++++++++++++++++++++ .github/workflows/tests.yml | 75 +----------- 5 files changed, 175 insertions(+), 90 deletions(-) create mode 100644 .github/actions/setup-native/action.yml create mode 100644 .github/workflows/test_native.yml diff --git a/.github/actions/setup-native/action.yml b/.github/actions/setup-native/action.yml new file mode 100644 index 000000000..36c95d943 --- /dev/null +++ b/.github/actions/setup-native/action.yml @@ -0,0 +1,14 @@ +name: Setup native build +description: Install libraries needed for building the Native/Portduino build + +runs: + using: composite + steps: + - name: Setup base + id: base + uses: ./.github/actions/setup-base + + - name: Install libs needed for native build + shell: bash + run: | + sudo apt-get install -y libbluetooth-dev libgpiod-dev libyaml-cpp-dev openssl libssl-dev libulfius-dev liborcania-dev libusb-1.0-0-dev libi2c-dev diff --git a/.github/workflows/build_native.yml b/.github/workflows/build_native.yml index 74bc074aa..11ba09ad7 100644 --- a/.github/workflows/build_native.yml +++ b/.github/workflows/build_native.yml @@ -10,12 +10,6 @@ jobs: build-native: runs-on: ubuntu-latest steps: - - name: Install libs needed for native build - shell: bash - run: | - sudo apt-get update --fix-missing - sudo apt-get install -y libbluetooth-dev libgpiod-dev libyaml-cpp-dev openssl libssl-dev libulfius-dev liborcania-dev libusb-1.0-0-dev libi2c-dev - - name: Checkout code uses: actions/checkout@v4 with: @@ -23,17 +17,9 @@ jobs: ref: ${{github.event.pull_request.head.ref}} repository: ${{github.event.pull_request.head.repo.full_name}} - - name: Upgrade python tools - shell: bash - run: | - python -m pip install --upgrade pip - pip install -U platformio adafruit-nrfutil - pip install -U meshtastic --pre - - - name: Upgrade platformio - shell: bash - run: | - pio upgrade + - name: Setup native build + id: base + uses: ./.github/actions/setup-native - name: Build Native run: bin/build-native.sh diff --git a/.github/workflows/main_matrix.yml b/.github/workflows/main_matrix.yml index 0109bef1a..a437411b5 100644 --- a/.github/workflows/main_matrix.yml +++ b/.github/workflows/main_matrix.yml @@ -137,6 +137,9 @@ jobs: package-native: uses: ./.github/workflows/package_amd64.yml + test-native: + uses: ./.github/workflows/test_native.yml + build-docker: if: ${{ github.event_name == 'workflow_dispatch' }} uses: ./.github/workflows/build_docker.yml diff --git a/.github/workflows/test_native.yml b/.github/workflows/test_native.yml new file mode 100644 index 000000000..8fe4e1c03 --- /dev/null +++ b/.github/workflows/test_native.yml @@ -0,0 +1,153 @@ +name: Run Tests on Native platform + +on: + workflow_call: + workflow_dispatch: + +permissions: {} + +env: + LCOV_CAPTURE_FLAGS: --quiet --capture --include "${PWD}/src/*" --exclude '*/src/mesh/generated/*' --directory .pio/build/coverage/src --base-directory "${PWD}" + +jobs: + simulator-tests: + name: Native Simulator Tests + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + submodules: recursive + + - name: Setup native build + id: base + uses: ./.github/actions/setup-native + + - name: Install simulator dependencies + run: pip install -U dotmap + + # We now run integration test before other build steps (to quickly see runtime failures) + - name: Build for native/coverage + run: platformio run -e coverage + + - name: Capture initial coverage information + shell: bash + run: | + sudo apt-get install -y lcov + lcov ${{ env.LCOV_CAPTURE_FLAGS }} --initial --output-file coverage_base.info + + - name: Integration test + run: | + .pio/build/coverage/program & + PID=$! + timeout 20 bash -c "until ls -al /proc/$PID/fd | grep socket; do sleep 1; done" + echo "Simulator started, launching python test..." + python3 -c 'from meshtastic.test import testSimulator; testSimulator()' + wait + + - name: Capture coverage information + if: always() # run this step even if previous step failed + run: lcov ${{ env.LCOV_CAPTURE_FLAGS }} --test-name integration --output-file coverage_integration.info + + - name: Get release version string + if: always() # run this step even if previous step failed + run: echo "version=$(./bin/buildinfo.py long)" >> $GITHUB_OUTPUT + id: version + + - name: Save coverage information + uses: actions/upload-artifact@v4 + if: always() # run this step even if previous step failed + with: + name: lcov-coverage-info-native-simulator-test-${{ steps.version.outputs.version }}.zip + overwrite: true + path: ./coverage_*.info + + platformio-tests: + name: Native PlatformIO Tests + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + submodules: recursive + + - name: Setup native build + id: base + uses: ./.github/actions/setup-native + + - name: Get release version string + run: echo "version=$(./bin/buildinfo.py long)" >> $GITHUB_OUTPUT + id: version + + - name: PlatformIO Tests + run: platformio test -e coverage --junit-output-path testreport.xml + + - name: Save test results + if: always() # run this step even if previous step failed + uses: actions/upload-artifact@v4 + with: + name: platformio-test-report-${{ steps.version.outputs.version }}.zip + overwrite: true + path: ./testreport.xml + + - name: Capture coverage information + if: always() # run this step even if previous step failed + run: | + sudo apt-get install -y lcov + lcov ${{ env.LCOV_CAPTURE_FLAGS }} --test-name tests --output-file coverage_tests.info + + - name: Save coverage information + uses: actions/upload-artifact@v4 + if: always() # run this step even if previous step failed + with: + name: lcov-coverage-info-native-platformio-tests-${{ steps.version.outputs.version }}.zip + overwrite: true + path: ./coverage_*.info + + generate-reports: + name: Generate Test Reports + runs-on: ubuntu-latest + permissions: # Needed for dorny/test-reporter. + contents: read + actions: read + checks: write + needs: + - simulator-tests + - platformio-tests + if: always() + steps: + - uses: actions/checkout@v4 + + - name: Get release version string + run: echo "version=$(./bin/buildinfo.py long)" >> $GITHUB_OUTPUT + id: version + + - name: Download test artifacts + uses: actions/download-artifact@v4 + with: + name: platformio-test-report-${{ steps.version.outputs.version }}.zip + merge-multiple: true + + - name: Test Report + uses: dorny/test-reporter@v1.9.1 + with: + name: PlatformIO Tests + path: testreport.xml + reporter: java-junit + + - name: Download coverage artifacts + uses: actions/download-artifact@v4 + with: + pattern: lcov-coverage-info-native-*-${{ steps.version.outputs.version }}.zip + path: code-coverage-report + merge-multiple: true + + - name: Generate Code Coverage Report + run: | + sudo apt-get install -y lcov + lcov --quiet --add-tracefile code-coverage-report/coverage_base.info --add-tracefile code-coverage-report/coverage_integration.info --add-tracefile code-coverage-report/coverage_tests.info --output-file code-coverage-report/coverage_src.info + genhtml --quiet --legend --prefix "${PWD}" code-coverage-report/coverage_src.info --output-directory code-coverage-report + + - name: Save Code Coverage Report + uses: actions/upload-artifact@v4 + with: + name: code-coverage-report-${{ steps.version.outputs.version }}.zip + path: code-coverage-report diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index ae9f82543..c9489db1a 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -6,79 +6,8 @@ on: workflow_dispatch: {} jobs: - test-simulator: - runs-on: ubuntu-latest - env: - LCOV_CAPTURE_FLAGS: --quiet --capture --include "${PWD}/src/*" --exclude '*/src/mesh/generated/*' --directory .pio/build/coverage/src --base-directory "${PWD}" - steps: - - name: Install libs needed for native build - shell: bash - run: | - sudo apt-get update --fix-missing - sudo apt-get install -y libbluetooth-dev libgpiod-dev libyaml-cpp-dev openssl libssl-dev libulfius-dev liborcania-dev libusb-1.0-0-dev libi2c-dev - sudo apt-get install -y lcov - - - name: Checkout code - uses: actions/checkout@v4 - with: - submodules: recursive - - - name: Upgrade python tools - shell: bash - run: | - python -m pip install --upgrade pip - pip install -U platformio adafruit-nrfutil dotmap - pip install -U meshtastic --pre - - - name: Upgrade platformio - shell: bash - run: | - pio upgrade - - - name: Build Native - run: bin/build-native.sh - - # We now run integration test before other build steps (to quickly see runtime failures) - - name: Build for native/coverage - run: | - platformio run -e coverage - lcov ${{ env.LCOV_CAPTURE_FLAGS }} --initial --output-file coverage_base.info - - - name: Integration test - run: | - .pio/build/coverage/program & - PID=$! - timeout 20 bash -c "until ls -al /proc/$PID/fd | grep socket; do sleep 1; done" - echo "Simulator started, launching python test..." - python3 -c 'from meshtastic.test import testSimulator; testSimulator()' - wait - lcov ${{ env.LCOV_CAPTURE_FLAGS }} --test-name integration --output-file coverage_integration.info - - - name: PlatformIO Tests - run: | - platformio test -e coverage --junit-output-path testreport.xml - lcov ${{ env.LCOV_CAPTURE_FLAGS }} --test-name tests --output-file coverage_tests.info - - - name: Test Report - uses: dorny/test-reporter@v1.9.1 - if: success() || failure() # run this step even if previous step failed - with: - name: PlatformIO Tests - path: testreport.xml - reporter: java-junit - - - name: Generate Code Coverage Report - run: | - lcov --quiet --add-tracefile coverage_base.info --add-tracefile coverage_integration.info --add-tracefile coverage_tests.info --output-file coverage_src.info - mkdir code-coverage-report - genhtml --quiet --legend --prefix "${PWD}" coverage_src.info --output-directory code-coverage-report - mv coverage_*.info code-coverage-report - - - name: Save Code Coverage Report - uses: actions/upload-artifact@v4 - with: - name: code-coverage-report - path: code-coverage-report + native-tests: + uses: ./.github/workflows/test_native.yml hardware-tests: runs-on: test-runner From 88d8ab53c8fba2830a71c9c0131b990d0f0f4e64 Mon Sep 17 00:00:00 2001 From: Eric Severance Date: Wed, 1 Jan 2025 19:37:11 -0800 Subject: [PATCH 04/11] Disable coverage generation (#5719) * Disable coverage generation * Comment a bit more of the report generation --- .github/workflows/test_native.yml | 54 +++++++++++++++---------------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/.github/workflows/test_native.yml b/.github/workflows/test_native.yml index 8fe4e1c03..9de3b34a6 100644 --- a/.github/workflows/test_native.yml +++ b/.github/workflows/test_native.yml @@ -120,34 +120,34 @@ jobs: run: echo "version=$(./bin/buildinfo.py long)" >> $GITHUB_OUTPUT id: version - - name: Download test artifacts - uses: actions/download-artifact@v4 - with: - name: platformio-test-report-${{ steps.version.outputs.version }}.zip - merge-multiple: true + # - name: Download test artifacts + # uses: actions/download-artifact@v4 + # with: + # name: platformio-test-report-${{ steps.version.outputs.version }}.zip + # merge-multiple: true - - name: Test Report - uses: dorny/test-reporter@v1.9.1 - with: - name: PlatformIO Tests - path: testreport.xml - reporter: java-junit + # - name: Test Report + # uses: dorny/test-reporter@v1.9.1 + # with: + # name: PlatformIO Tests + # path: testreport.xml + # reporter: java-junit - - name: Download coverage artifacts - uses: actions/download-artifact@v4 - with: - pattern: lcov-coverage-info-native-*-${{ steps.version.outputs.version }}.zip - path: code-coverage-report - merge-multiple: true + # - name: Download coverage artifacts + # uses: actions/download-artifact@v4 + # with: + # pattern: lcov-coverage-info-native-*-${{ steps.version.outputs.version }}.zip + # path: code-coverage-report + # merge-multiple: true - - name: Generate Code Coverage Report - run: | - sudo apt-get install -y lcov - lcov --quiet --add-tracefile code-coverage-report/coverage_base.info --add-tracefile code-coverage-report/coverage_integration.info --add-tracefile code-coverage-report/coverage_tests.info --output-file code-coverage-report/coverage_src.info - genhtml --quiet --legend --prefix "${PWD}" code-coverage-report/coverage_src.info --output-directory code-coverage-report + # - name: Generate Code Coverage Report + # run: | + # sudo apt-get install -y lcov + # lcov --quiet --add-tracefile code-coverage-report/coverage_base.info --add-tracefile code-coverage-report/coverage_integration.info --add-tracefile code-coverage-report/coverage_tests.info --output-file code-coverage-report/coverage_src.info + # genhtml --quiet --legend --prefix "${PWD}" code-coverage-report/coverage_src.info --output-directory code-coverage-report - - name: Save Code Coverage Report - uses: actions/upload-artifact@v4 - with: - name: code-coverage-report-${{ steps.version.outputs.version }}.zip - path: code-coverage-report + # - name: Save Code Coverage Report + # uses: actions/upload-artifact@v4 + # with: + # name: code-coverage-report-${{ steps.version.outputs.version }}.zip + # path: code-coverage-report From 7a1c32b89aa348871ff2e97ebf7a618f8274309e Mon Sep 17 00:00:00 2001 From: Eric Severance Date: Wed, 1 Jan 2025 20:41:13 -0800 Subject: [PATCH 05/11] test_native.yaml checks out code for the PR. (#5720) --- .github/workflows/test_native.yml | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/.github/workflows/test_native.yml b/.github/workflows/test_native.yml index 9de3b34a6..1cc1f7c40 100644 --- a/.github/workflows/test_native.yml +++ b/.github/workflows/test_native.yml @@ -16,6 +16,8 @@ jobs: steps: - uses: actions/checkout@v4 with: + ref: ${{github.event.pull_request.head.ref}} + repository: ${{github.event.pull_request.head.repo.full_name}} submodules: recursive - name: Setup native build @@ -67,6 +69,8 @@ jobs: steps: - uses: actions/checkout@v4 with: + ref: ${{github.event.pull_request.head.ref}} + repository: ${{github.event.pull_request.head.repo.full_name}} submodules: recursive - name: Setup native build @@ -115,23 +119,26 @@ jobs: if: always() steps: - uses: actions/checkout@v4 + with: + ref: ${{github.event.pull_request.head.ref}} + repository: ${{github.event.pull_request.head.repo.full_name}} - name: Get release version string run: echo "version=$(./bin/buildinfo.py long)" >> $GITHUB_OUTPUT id: version - # - name: Download test artifacts - # uses: actions/download-artifact@v4 - # with: - # name: platformio-test-report-${{ steps.version.outputs.version }}.zip - # merge-multiple: true + - name: Download test artifacts + uses: actions/download-artifact@v4 + with: + name: platformio-test-report-${{ steps.version.outputs.version }}.zip + merge-multiple: true - # - name: Test Report - # uses: dorny/test-reporter@v1.9.1 - # with: - # name: PlatformIO Tests - # path: testreport.xml - # reporter: java-junit + - name: Test Report + uses: dorny/test-reporter@v1.9.1 + with: + name: PlatformIO Tests + path: testreport.xml + reporter: java-junit # - name: Download coverage artifacts # uses: actions/download-artifact@v4 From 93e2bc7058ba0d0b7b3936590b5c185bc418fba9 Mon Sep 17 00:00:00 2001 From: Eric Severance Date: Wed, 1 Jan 2025 22:53:07 -0800 Subject: [PATCH 06/11] Use relative paths in coverage info files (#5721) --- .github/workflows/test_native.yml | 38 +++++++++++++++++-------------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/.github/workflows/test_native.yml b/.github/workflows/test_native.yml index 1cc1f7c40..8c1527279 100644 --- a/.github/workflows/test_native.yml +++ b/.github/workflows/test_native.yml @@ -36,6 +36,7 @@ jobs: run: | sudo apt-get install -y lcov lcov ${{ env.LCOV_CAPTURE_FLAGS }} --initial --output-file coverage_base.info + sed -i -e "s#${PWD}#.#" coverage_base.info # Make paths relative. - name: Integration test run: | @@ -48,7 +49,9 @@ jobs: - name: Capture coverage information if: always() # run this step even if previous step failed - run: lcov ${{ env.LCOV_CAPTURE_FLAGS }} --test-name integration --output-file coverage_integration.info + run: | + lcov ${{ env.LCOV_CAPTURE_FLAGS }} --test-name integration --output-file coverage_integration.info + sed -i -e "s#${PWD}#.#" coverage_integration.info # Make paths relative. - name: Get release version string if: always() # run this step even if previous step failed @@ -97,6 +100,7 @@ jobs: run: | sudo apt-get install -y lcov lcov ${{ env.LCOV_CAPTURE_FLAGS }} --test-name tests --output-file coverage_tests.info + sed -i -e "s#${PWD}#.#" coverage_tests.info # Make paths relative. - name: Save coverage information uses: actions/upload-artifact@v4 @@ -140,21 +144,21 @@ jobs: path: testreport.xml reporter: java-junit - # - name: Download coverage artifacts - # uses: actions/download-artifact@v4 - # with: - # pattern: lcov-coverage-info-native-*-${{ steps.version.outputs.version }}.zip - # path: code-coverage-report - # merge-multiple: true + - name: Download coverage artifacts + uses: actions/download-artifact@v4 + with: + pattern: lcov-coverage-info-native-*-${{ steps.version.outputs.version }}.zip + path: code-coverage-report + merge-multiple: true - # - name: Generate Code Coverage Report - # run: | - # sudo apt-get install -y lcov - # lcov --quiet --add-tracefile code-coverage-report/coverage_base.info --add-tracefile code-coverage-report/coverage_integration.info --add-tracefile code-coverage-report/coverage_tests.info --output-file code-coverage-report/coverage_src.info - # genhtml --quiet --legend --prefix "${PWD}" code-coverage-report/coverage_src.info --output-directory code-coverage-report + - name: Generate Code Coverage Report + run: | + sudo apt-get install -y lcov + lcov --quiet --add-tracefile code-coverage-report/coverage_base.info --add-tracefile code-coverage-report/coverage_integration.info --add-tracefile code-coverage-report/coverage_tests.info --output-file code-coverage-report/coverage_src.info + genhtml --quiet --legend --prefix "${PWD}" code-coverage-report/coverage_src.info --output-directory code-coverage-report - # - name: Save Code Coverage Report - # uses: actions/upload-artifact@v4 - # with: - # name: code-coverage-report-${{ steps.version.outputs.version }}.zip - # path: code-coverage-report + - name: Save Code Coverage Report + uses: actions/upload-artifact@v4 + with: + name: code-coverage-report-${{ steps.version.outputs.version }}.zip + path: code-coverage-report From 9f7cbf1b4fe61979c5e98f9affb1d801af7147c8 Mon Sep 17 00:00:00 2001 From: Eric Severance Date: Thu, 2 Jan 2025 03:32:39 -0800 Subject: [PATCH 07/11] MQTT unit test can inject WiFiClient (#5716) --- src/mqtt/MQTT.cpp | 12 +++++++----- src/mqtt/MQTT.h | 40 +++++++++++++++++++++++----------------- 2 files changed, 30 insertions(+), 22 deletions(-) diff --git a/src/mqtt/MQTT.cpp b/src/mqtt/MQTT.cpp index 5141af560..46fb607b5 100644 --- a/src/mqtt/MQTT.cpp +++ b/src/mqtt/MQTT.cpp @@ -282,7 +282,9 @@ void mqttInit() } #if HAS_NETWORKING -MQTT::MQTT() : concurrency::OSThread("mqtt"), pubSub(mqttClient), mqttQueue(MAX_MQTT_QUEUE) +MQTT::MQTT() : MQTT(std::unique_ptr(new MQTTClient())) {} +MQTT::MQTT(std::unique_ptr _mqttClient) + : concurrency::OSThread("mqtt"), mqttQueue(MAX_MQTT_QUEUE), mqttClient(std::move(_mqttClient)), pubSub(*mqttClient) #else MQTT::MQTT() : concurrency::OSThread("mqtt"), mqttQueue(MAX_MQTT_QUEUE) #endif @@ -420,13 +422,13 @@ void MQTT::reconnect() } } else { LOG_INFO("Use non-TLS-encrypted session"); - pubSub.setClient(mqttClient); + pubSub.setClient(*mqttClient); } #else - pubSub.setClient(mqttClient); + pubSub.setClient(*mqttClient); #endif #elif HAS_NETWORKING - pubSub.setClient(mqttClient); + pubSub.setClient(*mqttClient); #endif std::pair hostAndPort = parseHostAndPort(serverAddr, serverPort); @@ -444,7 +446,7 @@ void MQTT::reconnect() enabled = true; // Start running background process again runASAP = true; reconnectCount = 0; - isMqttServerAddressPrivate = isPrivateIpAddress(mqttClient.remoteIP()); + isMqttServerAddressPrivate = isPrivateIpAddress(mqttClient->remoteIP()); publishNodeInfo(); sendSubscriptions(); diff --git a/src/mqtt/MQTT.h b/src/mqtt/MQTT.h index cb1fffcc9..cf52ad877 100644 --- a/src/mqtt/MQTT.h +++ b/src/mqtt/MQTT.h @@ -22,6 +22,7 @@ #if HAS_NETWORKING #include +#include #endif #define MAX_MQTT_QUEUE 16 @@ -32,24 +33,7 @@ */ class MQTT : private concurrency::OSThread { - // supposedly the current version is busted: - // http://www.iotsharing.com/2017/08/how-to-use-esp32-mqtts-with-mqtts-mosquitto-broker-tls-ssl.html -#if HAS_WIFI - WiFiClient mqttClient; -#if !defined(ARCH_PORTDUINO) -#if (defined(ESP_ARDUINO_VERSION_MAJOR) && ESP_ARDUINO_VERSION_MAJOR < 3) || defined(RPI_PICO) - WiFiClientSecure wifiSecureClient; -#endif -#endif -#endif -#if HAS_ETHERNET - EthernetClient mqttClient; -#endif - public: -#if HAS_NETWORKING - PubSubClient pubSub; -#endif MQTT(); /** @@ -93,7 +77,29 @@ class MQTT : private concurrency::OSThread virtual int32_t runOnce() override; +#ifndef PIO_UNIT_TESTING private: +#endif + // supposedly the current version is busted: + // http://www.iotsharing.com/2017/08/how-to-use-esp32-mqtts-with-mqtts-mosquitto-broker-tls-ssl.html +#if HAS_WIFI + using MQTTClient = WiFiClient; +#if !defined(ARCH_PORTDUINO) +#if (defined(ESP_ARDUINO_VERSION_MAJOR) && ESP_ARDUINO_VERSION_MAJOR < 3) || defined(RPI_PICO) + WiFiClientSecure wifiSecureClient; +#endif +#endif +#endif +#if HAS_ETHERNET + using MQTTClient = EthernetClient; +#endif + +#if HAS_NETWORKING + std::unique_ptr mqttClient; + PubSubClient pubSub; + explicit MQTT(std::unique_ptr mqttClient); +#endif + std::string cryptTopic = "/2/e/"; // msh/2/e/CHANNELID/NODEID std::string jsonTopic = "/2/json/"; // msh/2/json/CHANNELID/NODEID std::string mapTopic = "/2/map/"; // For protobuf-encoded MapReport messages From 9bda080e3d0be0f025f88d36bb4a9aa605954fc3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Thomas=20G=C3=B6ttgens?= Date: Thu, 2 Jan 2025 16:05:12 +0100 Subject: [PATCH 08/11] evaluate GPS_THREAD_INTERVAL after variant file (#5722) --- src/configuration.h | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/configuration.h b/src/configuration.h index 994f1e72e..2c77b55e3 100644 --- a/src/configuration.h +++ b/src/configuration.h @@ -178,13 +178,6 @@ along with this program. If not, see . #define TCA9535_ADDR 0x20 #define TCA9555_ADDR 0x26 -// ----------------------------------------------------------------------------- -// GPS -// ----------------------------------------------------------------------------- -#ifndef GPS_THREAD_INTERVAL -#define GPS_THREAD_INTERVAL 200 -#endif - // ----------------------------------------------------------------------------- // Touchscreen // ----------------------------------------------------------------------------- @@ -206,6 +199,10 @@ along with this program. If not, see . #define VEXT_ON_VALUE LOW #endif +// ----------------------------------------------------------------------------- +// GPS +// ----------------------------------------------------------------------------- + #ifndef GPS_BAUDRATE #define GPS_BAUDRATE 9600 #define GPS_BAUDRATE_FIXED 0 @@ -213,6 +210,10 @@ along with this program. If not, see . #define GPS_BAUDRATE_FIXED 1 #endif +#ifndef GPS_THREAD_INTERVAL +#define GPS_THREAD_INTERVAL 200 +#endif + /* Step #2: follow with defines common to the architecture; also enable HAS_ option not specifically disabled by variant.h */ #include "architecture.h" From b41efc17ba60a0f4b93cf85fe0c1b08070aeb67e Mon Sep 17 00:00:00 2001 From: Eric Severance Date: Thu, 2 Jan 2025 08:32:38 -0800 Subject: [PATCH 09/11] Disable BUILD_EPOCH for unit tests (#5723) --- .github/workflows/test_native.yml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/.github/workflows/test_native.yml b/.github/workflows/test_native.yml index 8c1527279..d016635ef 100644 --- a/.github/workflows/test_native.yml +++ b/.github/workflows/test_native.yml @@ -84,6 +84,11 @@ jobs: run: echo "version=$(./bin/buildinfo.py long)" >> $GITHUB_OUTPUT id: version + # Disable (comment-out) BUILD_EPOCH. It causes a full rebuild between tests and resets the + # coverage information each time. + - name: Disable BUILD_EPOCH + run: sed -i 's/-DBUILD_EPOCH=$UNIX_TIME/#-DBUILD_EPOCH=$UNIX_TIME/' platformio.ini + - name: PlatformIO Tests run: platformio test -e coverage --junit-output-path testreport.xml From 9d710041c43af266dc0e32703e39775bb1f4929e Mon Sep 17 00:00:00 2001 From: Tom Fifield Date: Fri, 3 Jan 2025 09:01:10 +0800 Subject: [PATCH 10/11] Add MESHTASTIC_EXCLUDE_SOCKETAPI (#5729) MESHTASTIC_EXCLUDE_SOCKETAPI disables the API Server when set. Co-authored-by: mverch67 Co-authored-by: GUVWAF --- src/mesh/eth/ethClient.cpp | 3 ++- src/mesh/wifi/WiFiAPClient.cpp | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/mesh/eth/ethClient.cpp b/src/mesh/eth/ethClient.cpp index 08ecb7ce8..24c4f0db1 100644 --- a/src/mesh/eth/ethClient.cpp +++ b/src/mesh/eth/ethClient.cpp @@ -66,8 +66,9 @@ static int32_t reconnectETH() syslog.enable(); } - // initWebServer(); +#if !MESHTASTIC_EXCLUDE_SOCKETAPI initApiServer(); +#endif ethStartupComplete = true; } diff --git a/src/mesh/wifi/WiFiAPClient.cpp b/src/mesh/wifi/WiFiAPClient.cpp index 38aa2e2a2..2f8138921 100644 --- a/src/mesh/wifi/WiFiAPClient.cpp +++ b/src/mesh/wifi/WiFiAPClient.cpp @@ -106,7 +106,9 @@ static void onNetworkConnected() #if defined(ARCH_ESP32) && !MESHTASTIC_EXCLUDE_WEBSERVER initWebServer(); #endif +#if !MESHTASTIC_EXCLUDE_SOCKETAPI initApiServer(); +#endif APStartupComplete = true; } From e1aaafb77a9f6e0bc159242de24b9b3cbb9e1782 Mon Sep 17 00:00:00 2001 From: Tom Fifield Date: Fri, 3 Jan 2025 10:05:26 +0800 Subject: [PATCH 11/11] Cherrypick "add more locking for shared SPI devices (#5595) " (#5728) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * add more locking for shared SPI devices (#5595) * add more locking for shared SPI devices * call initSPI before the lock is used * remove old one * don't double lock * Add missing unlock * More missing unlocks * Add locks to SafeFile, remove from `readcb`, introduce some LockGuards * fix lock in setupSDCard() * pull radiolib trunk with SPI-CS fixes * change ContentHandler to Constructor type locks, where applicable --------- Co-authored-by: mverch67 Co-authored-by: GUVWAF Co-authored-by: Manuel <71137295+mverch67@users.noreply.github.com> * mesh-tab: lower I2C touch frequency --------- Co-authored-by: Thomas Göttgens Co-authored-by: mverch67 Co-authored-by: GUVWAF Co-authored-by: Manuel <71137295+mverch67@users.noreply.github.com> --- platformio.ini | 3 ++- src/FSCommon.cpp | 21 ++++++++++++++-- src/SafeFile.cpp | 15 ++++++++---- src/SafeFile.h | 1 + src/main.cpp | 4 ++-- src/mesh/NodeDB.cpp | 22 +++++++++++++---- src/mesh/PhoneAPI.cpp | 3 +++ src/mesh/http/ContentHandler.cpp | 15 ++++++++++++ src/mesh/mesh-pb-constants.cpp | 8 +++++-- src/modules/AdminModule.cpp | 4 ++++ src/modules/CannedMessageModule.cpp | 7 ++++-- src/modules/RangeTestModule.cpp | 2 ++ src/modules/Telemetry/Sensor/BME680Sensor.cpp | 5 ++++ .../Telemetry/Sensor/NAU7802Sensor.cpp | 7 +++++- src/xmodem.cpp | 24 +++++++++++++++++++ variants/mesh-tab/platformio.ini | 6 ++--- 16 files changed, 125 insertions(+), 22 deletions(-) diff --git a/platformio.ini b/platformio.ini index 3129decaa..cd32ed179 100644 --- a/platformio.ini +++ b/platformio.ini @@ -124,7 +124,8 @@ lib_deps = [radiolib_base] lib_deps = - jgromes/RadioLib@7.1.0 + ; jgromes/RadioLib@7.1.0 + https://github.com/jgromes/RadioLib.git#92b687821ff4e6c358d866f84566f66672ab02b8 ; Common libs for environmental measurements in telemetry module ; (not included in native / portduino) diff --git a/src/FSCommon.cpp b/src/FSCommon.cpp index df46c1941..6d8ff835c 100644 --- a/src/FSCommon.cpp +++ b/src/FSCommon.cpp @@ -9,6 +9,7 @@ * */ #include "FSCommon.h" +#include "SPILock.h" #include "configuration.h" #ifdef HAS_SDCARD @@ -102,6 +103,8 @@ bool copyFile(const char *from, const char *to) return true; #elif defined(FSCom) + // take SPI Lock + concurrency::LockGuard g(spiLock); unsigned char cbuffer[16]; File f1 = FSCom.open(from, FILE_O_READ); @@ -145,16 +148,23 @@ bool renameFile(const char *pathFrom, const char *pathTo) return false; } #elif defined(FSCom) + #ifdef ARCH_ESP32 + // take SPI Lock + spiLock->lock(); // rename was fixed for ESP32 IDF LittleFS in April - return FSCom.rename(pathFrom, pathTo); + bool result = FSCom.rename(pathFrom, pathTo); + spiLock->unlock(); + return result; #else + // copyFile does its own locking. if (copyFile(pathFrom, pathTo) && FSCom.remove(pathFrom)) { return true; } else { return false; } #endif + #endif } @@ -164,6 +174,7 @@ bool renameFile(const char *pathFrom, const char *pathTo) * @brief Get the list of files in a directory. * * This function returns a list of files in a directory. The list includes the full path of each file. + * We can't use SPILOCK here because of recursion. Callers of this function should use SPILOCK. * * @param dirname The name of the directory. * @param levels The number of levels of subdirectories to list. @@ -212,6 +223,7 @@ std::vector getFiles(const char *dirname, uint8_t levels) /** * Lists the contents of a directory. + * We can't use SPILOCK here because of recursion. Callers of this function should use SPILOCK. * * @param dirname The name of the directory to list. * @param levels The number of levels of subdirectories to list. @@ -325,18 +337,21 @@ void listDir(const char *dirname, uint8_t levels, bool del) void rmDir(const char *dirname) { #ifdef FSCom + #if (defined(ARCH_ESP32) || defined(ARCH_RP2040) || defined(ARCH_PORTDUINO)) listDir(dirname, 10, true); #elif defined(ARCH_NRF52) // nRF52 implementation of LittleFS has a recursive delete function FSCom.rmdir_r(dirname); #endif + #endif } void fsInit() { #ifdef FSCom + spiLock->lock(); if (!FSBegin()) { LOG_ERROR("Filesystem mount failed"); // assert(0); This auto-formats the partition, so no need to fail here. @@ -347,6 +362,7 @@ void fsInit() LOG_DEBUG("Filesystem files:"); #endif listDir("/", 10); + spiLock->unlock(); #endif } @@ -356,6 +372,7 @@ void fsInit() void setupSDCard() { #ifdef HAS_SDCARD + concurrency::LockGuard g(spiLock); SDHandler.begin(SPI_SCK, SPI_MISO, SPI_MOSI); if (!SD.begin(SDCARD_CS, SDHandler)) { @@ -383,4 +400,4 @@ void setupSDCard() LOG_DEBUG("Total space: %lu MB", (uint32_t)(SD.totalBytes() / (1024 * 1024))); LOG_DEBUG("Used space: %lu MB", (uint32_t)(SD.usedBytes() / (1024 * 1024))); #endif -} \ No newline at end of file +} diff --git a/src/SafeFile.cpp b/src/SafeFile.cpp index c76ff8054..c5d7b335e 100644 --- a/src/SafeFile.cpp +++ b/src/SafeFile.cpp @@ -5,6 +5,7 @@ // Only way to work on both esp32 and nrf52 static File openFile(const char *filename, bool fullAtomic) { + concurrency::LockGuard g(spiLock); if (!fullAtomic) FSCom.remove(filename); // Nuke the old file to make space (ignore if it !exists) @@ -53,14 +54,19 @@ bool SafeFile::close() if (!f) return false; + spiLock->lock(); f.close(); + spiLock->unlock(); if (!testReadback()) return false; - // brief window of risk here ;-) - if (fullAtomic && FSCom.exists(filename.c_str()) && !FSCom.remove(filename.c_str())) { - LOG_ERROR("Can't remove old pref file"); - return false; + { // Scope for lock + concurrency::LockGuard g(spiLock); + // brief window of risk here ;-) + if (fullAtomic && FSCom.exists(filename.c_str()) && !FSCom.remove(filename.c_str())) { + LOG_ERROR("Can't remove old pref file"); + return false; + } } String filenameTmp = filename; @@ -76,6 +82,7 @@ bool SafeFile::close() /// Read our (closed) tempfile back in and compare the hash bool SafeFile::testReadback() { + concurrency::LockGuard g(spiLock); bool lfs_failed = lfs_assert_failed; lfs_assert_failed = false; diff --git a/src/SafeFile.h b/src/SafeFile.h index 61361d312..3d0f81cad 100644 --- a/src/SafeFile.h +++ b/src/SafeFile.h @@ -1,6 +1,7 @@ #pragma once #include "FSCommon.h" +#include "SPILock.h" #include "configuration.h" #ifdef FSCom diff --git a/src/main.cpp b/src/main.cpp index 5982e709d..c2b20b1c1 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -364,6 +364,8 @@ void setup() #endif #endif + initSPI(); + OSThread::setup(); ledPeriodic = new Periodic("Blink", ledBlinker); @@ -640,8 +642,6 @@ void setup() rp2040Setup(); #endif - initSPI(); // needed here before reading from littleFS - // We do this as early as possible because this loads preferences from flash // but we need to do this after main cpu init (esp32setup), because we need the random seed set nodeDB = new NodeDB; diff --git a/src/mesh/NodeDB.cpp b/src/mesh/NodeDB.cpp index bbc9eb1ea..8e084c99d 100644 --- a/src/mesh/NodeDB.cpp +++ b/src/mesh/NodeDB.cpp @@ -13,6 +13,7 @@ #include "PowerFSM.h" #include "RTC.h" #include "Router.h" +#include "SPILock.h" #include "SafeFile.h" #include "TypeConversions.h" #include "error.h" @@ -423,12 +424,15 @@ bool NodeDB::factoryReset(bool eraseBleBonds) { LOG_INFO("Perform factory reset!"); // first, remove the "/prefs" (this removes most prefs) - rmDir("/prefs"); + spiLock->lock(); + rmDir("/prefs"); // this uses spilock internally... + #ifdef FSCom if (FSCom.exists("/static/rangetest.csv") && !FSCom.remove("/static/rangetest.csv")) { LOG_ERROR("Could not remove rangetest.csv file"); } #endif + spiLock->unlock(); // second, install default state (this will deal with the duplicate mac address issue) installDefaultDeviceState(); installDefaultConfig(!eraseBleBonds); // Also preserve the private key if we're not erasing BLE bonds @@ -913,6 +917,7 @@ LoadFileResult NodeDB::loadProto(const char *filename, size_t protoSize, size_t { LoadFileResult state = LoadFileResult::OTHER_FAILURE; #ifdef FSCom + concurrency::LockGuard g(spiLock); auto f = FSCom.open(filename, FILE_O_READ); @@ -946,8 +951,10 @@ void NodeDB::loadFromDisk() // disk we will still factoryReset to restore things. #ifdef ARCH_ESP32 + spiLock->lock(); if (FSCom.exists("/static/static")) rmDir("/static/static"); // Remove bad static web files bundle from initial 2.5.13 release + spiLock->unlock(); #endif // static DeviceState scratch; We no longer read into a tempbuf because this structure is 15KB of valuable RAM @@ -1097,9 +1104,6 @@ void NodeDB::loadFromDisk() bool NodeDB::saveProto(const char *filename, size_t protoSize, const pb_msgdesc_t *fields, const void *dest_struct, bool fullAtomic) { -#ifdef ARCH_ESP32 - concurrency::LockGuard g(spiLock); -#endif bool okay = false; #ifdef FSCom auto f = SafeFile(filename, fullAtomic); @@ -1127,7 +1131,9 @@ bool NodeDB::saveProto(const char *filename, size_t protoSize, const pb_msgdesc_ bool NodeDB::saveChannelsToDisk() { #ifdef FSCom + spiLock->lock(); FSCom.mkdir("/prefs"); + spiLock->unlock(); #endif return saveProto(channelFileName, meshtastic_ChannelFile_size, &meshtastic_ChannelFile_msg, &channelFile); } @@ -1135,7 +1141,9 @@ bool NodeDB::saveChannelsToDisk() bool NodeDB::saveDeviceStateToDisk() { #ifdef FSCom + spiLock->lock(); FSCom.mkdir("/prefs"); + spiLock->unlock(); #endif // Note: if MAX_NUM_NODES=100 and meshtastic_NodeInfoLite_size=166, so will be approximately 17KB // Because so huge we _must_ not use fullAtomic, because the filesystem is probably too small to hold two copies of this @@ -1148,7 +1156,9 @@ bool NodeDB::saveToDiskNoRetry(int saveWhat) bool success = true; #ifdef FSCom + spiLock->lock(); FSCom.mkdir("/prefs"); + spiLock->unlock(); #endif if (saveWhat & SEGMENT_CONFIG) { config.has_device = true; @@ -1199,7 +1209,9 @@ bool NodeDB::saveToDisk(int saveWhat) if (!success) { LOG_ERROR("Failed to save to disk, retrying"); #ifdef ARCH_NRF52 // @geeksville is not ready yet to say we should do this on other platforms. See bug #4184 discussion + spiLock->lock(); FSCom.format(); + spiLock->unlock(); #endif success = saveToDiskNoRetry(saveWhat); @@ -1518,4 +1530,4 @@ void recordCriticalError(meshtastic_CriticalErrorCode code, uint32_t address, co LOG_ERROR("A critical failure occurred, portduino is exiting"); exit(2); #endif -} +} \ No newline at end of file diff --git a/src/mesh/PhoneAPI.cpp b/src/mesh/PhoneAPI.cpp index 8c1ba74c7..6789acbb3 100644 --- a/src/mesh/PhoneAPI.cpp +++ b/src/mesh/PhoneAPI.cpp @@ -12,6 +12,7 @@ #include "PhoneAPI.h" #include "PowerFSM.h" #include "RadioInterface.h" +#include "SPILock.h" #include "TypeConversions.h" #include "main.h" #include "xmodem.h" @@ -54,7 +55,9 @@ void PhoneAPI::handleStartConfig() // even if we were already connected - restart our state machine state = STATE_SEND_MY_INFO; pauseBluetoothLogging = true; + spiLock->lock(); filesManifest = getFiles("/", 10); + spiLock->unlock(); LOG_DEBUG("Got %d files in manifest", filesManifest.size()); LOG_INFO("Start API client config"); diff --git a/src/mesh/http/ContentHandler.cpp b/src/mesh/http/ContentHandler.cpp index aa8a68f91..5841fe478 100644 --- a/src/mesh/http/ContentHandler.cpp +++ b/src/mesh/http/ContentHandler.cpp @@ -10,6 +10,7 @@ #include "mesh/wifi/WiFiAPClient.h" #endif #include "Led.h" +#include "SPILock.h" #include "power.h" #include "serialization/JSON.h" #include @@ -236,6 +237,7 @@ void handleAPIv1ToRadio(HTTPRequest *req, HTTPResponse *res) void htmlDeleteDir(const char *dirname) { + File root = FSCom.open(dirname); if (!root) { return; @@ -318,6 +320,7 @@ void handleFsBrowseStatic(HTTPRequest *req, HTTPResponse *res) res->setHeader("Access-Control-Allow-Origin", "*"); res->setHeader("Access-Control-Allow-Methods", "GET"); + concurrency::LockGuard g(spiLock); auto fileList = htmlListDir("/static", 10); // create json output structure @@ -349,9 +352,12 @@ void handleFsDeleteStatic(HTTPRequest *req, HTTPResponse *res) res->setHeader("Content-Type", "application/json"); res->setHeader("Access-Control-Allow-Origin", "*"); res->setHeader("Access-Control-Allow-Methods", "DELETE"); + if (params->getQueryParameter("delete", paramValDelete)) { std::string pathDelete = "/" + paramValDelete; + concurrency::LockGuard g(spiLock); if (FSCom.remove(pathDelete.c_str())) { + LOG_INFO("%s", pathDelete.c_str()); JSONObject jsonObjOuter; jsonObjOuter["status"] = new JSONValue("ok"); @@ -360,6 +366,7 @@ void handleFsDeleteStatic(HTTPRequest *req, HTTPResponse *res) delete value; return; } else { + LOG_INFO("%s", pathDelete.c_str()); JSONObject jsonObjOuter; jsonObjOuter["status"] = new JSONValue("Error"); @@ -393,6 +400,8 @@ void handleStatic(HTTPRequest *req, HTTPResponse *res) filenameGzip = "/static/index.html.gz"; } + concurrency::LockGuard g(spiLock); + if (FSCom.exists(filename.c_str())) { file = FSCom.open(filename.c_str()); if (!file.available()) { @@ -410,6 +419,7 @@ void handleStatic(HTTPRequest *req, HTTPResponse *res) file = FSCom.open(filenameGzip.c_str()); res->setHeader("Content-Type", "text/html"); if (!file.available()) { + LOG_WARN("File not available - %s", filenameGzip.c_str()); res->println("Web server is running.

The content you are looking for can't be found. Please see: FAQ.

printf("

Saved %d bytes to %s

", (int)fileLength, pathname.c_str()); } if (!didwrite) { @@ -642,9 +654,11 @@ void handleReport(HTTPRequest *req, HTTPResponse *res) jsonObjMemory["heap_free"] = new JSONValue((int)memGet.getFreeHeap()); jsonObjMemory["psram_total"] = new JSONValue((int)memGet.getPsramSize()); jsonObjMemory["psram_free"] = new JSONValue((int)memGet.getFreePsram()); + spiLock->lock(); jsonObjMemory["fs_total"] = new JSONValue((int)FSCom.totalBytes()); jsonObjMemory["fs_used"] = new JSONValue((int)FSCom.usedBytes()); jsonObjMemory["fs_free"] = new JSONValue(int(FSCom.totalBytes() - FSCom.usedBytes())); + spiLock->unlock(); // data->power JSONObject jsonObjPower; @@ -786,6 +800,7 @@ void handleDeleteFsContent(HTTPRequest *req, HTTPResponse *res) LOG_INFO("Delete files from /static/* : "); + concurrency::LockGuard g(spiLock); htmlDeleteDir("/static"); res->println("

Back to admin"); diff --git a/src/mesh/mesh-pb-constants.cpp b/src/mesh/mesh-pb-constants.cpp index deb93d0a6..a8f4fd6d8 100644 --- a/src/mesh/mesh-pb-constants.cpp +++ b/src/mesh/mesh-pb-constants.cpp @@ -1,6 +1,7 @@ #include "configuration.h" #include "FSCommon.h" +#include "SPILock.h" #include "mesh-pb-constants.h" #include #include @@ -55,9 +56,12 @@ bool readcb(pb_istream_t *stream, uint8_t *buf, size_t count) /// Write to an arduino file bool writecb(pb_ostream_t *stream, const uint8_t *buf, size_t count) { + spiLock->lock(); auto file = (Print *)stream->state; // LOG_DEBUG("writing %d bytes to protobuf file", count); - return file->write(buf, count) == count; + bool status = file->write(buf, count) == count; + spiLock->unlock(); + return status; } #endif @@ -68,4 +72,4 @@ bool is_in_helper(uint32_t n, const uint32_t *array, pb_size_t count) return true; return false; -} +} \ No newline at end of file diff --git a/src/modules/AdminModule.cpp b/src/modules/AdminModule.cpp index fc3b914e5..7906b410b 100644 --- a/src/modules/AdminModule.cpp +++ b/src/modules/AdminModule.cpp @@ -4,6 +4,7 @@ #include "NodeDB.h" #include "PowerFSM.h" #include "RTC.h" +#include "SPILock.h" #include "meshUtils.h" #include #if defined(ARCH_ESP32) && !MESHTASTIC_EXCLUDE_BLUETOOTH @@ -358,12 +359,15 @@ bool AdminModule::handleReceivedProtobuf(const meshtastic_MeshPacket &mp, meshta } case meshtastic_AdminMessage_delete_file_request_tag: { LOG_DEBUG("Client requesting to delete file: %s", r->delete_file_request); + #ifdef FSCom + spiLock->lock(); if (FSCom.remove(r->delete_file_request)) { LOG_DEBUG("Successfully deleted file"); } else { LOG_DEBUG("Failed to delete file"); } + spiLock->unlock(); #endif break; } diff --git a/src/modules/CannedMessageModule.cpp b/src/modules/CannedMessageModule.cpp index c20b7b56e..5fb32fff5 100644 --- a/src/modules/CannedMessageModule.cpp +++ b/src/modules/CannedMessageModule.cpp @@ -9,6 +9,7 @@ #include "MeshService.h" #include "NodeDB.h" #include "PowerFSM.h" // needed for button bypass +#include "SPILock.h" #include "detect/ScanI2C.h" #include "input/ScanAndSelect.h" #include "mesh/generated/meshtastic/cannedmessages.pb.h" @@ -1184,8 +1185,10 @@ bool CannedMessageModule::saveProtoForModule() { bool okay = true; -#ifdef FS - FS.mkdir("/prefs"); +#ifdef FSCom + spiLock->lock(); + FSCom.mkdir("/prefs"); + spiLock->unlock(); #endif okay &= nodeDB->saveProto(cannedMessagesConfigFile, meshtastic_CannedMessageModuleConfig_size, diff --git a/src/modules/RangeTestModule.cpp b/src/modules/RangeTestModule.cpp index c42839d97..cad1d51f1 100644 --- a/src/modules/RangeTestModule.cpp +++ b/src/modules/RangeTestModule.cpp @@ -15,6 +15,7 @@ #include "PowerFSM.h" #include "RTC.h" #include "Router.h" +#include "SPILock.h" #include "airtime.h" #include "configuration.h" #include "gps/GeoCoord.h" @@ -205,6 +206,7 @@ bool RangeTestModuleRadio::appendFile(const meshtastic_MeshPacket &mp) LOG_DEBUG("gpsStatus->getDOP() %d", gpsStatus->getDOP()); LOG_DEBUG("-----------------------------------------"); */ + concurrency::LockGuard g(spiLock); if (!FSBegin()) { LOG_DEBUG("An Error has occurred while mounting the filesystem"); return 0; diff --git a/src/modules/Telemetry/Sensor/BME680Sensor.cpp b/src/modules/Telemetry/Sensor/BME680Sensor.cpp index 18515d0a8..9237cf0c9 100644 --- a/src/modules/Telemetry/Sensor/BME680Sensor.cpp +++ b/src/modules/Telemetry/Sensor/BME680Sensor.cpp @@ -5,6 +5,7 @@ #include "../mesh/generated/meshtastic/telemetry.pb.h" #include "BME680Sensor.h" #include "FSCommon.h" +#include "SPILock.h" #include "TelemetrySensor.h" BME680Sensor::BME680Sensor() : TelemetrySensor(meshtastic_TelemetrySensorType_BME680, "BME680") {} @@ -75,6 +76,7 @@ bool BME680Sensor::getMetrics(meshtastic_Telemetry *measurement) void BME680Sensor::loadState() { #ifdef FSCom + spiLock->lock(); auto file = FSCom.open(bsecConfigFileName, FILE_O_READ); if (file) { file.read((uint8_t *)&bsecState, BSEC_MAX_STATE_BLOB_SIZE); @@ -84,6 +86,7 @@ void BME680Sensor::loadState() } else { LOG_INFO("No %s state found (File: %s)", sensorName, bsecConfigFileName); } + spiLock->unlock(); #else LOG_ERROR("ERROR: Filesystem not implemented"); #endif @@ -92,6 +95,7 @@ void BME680Sensor::loadState() void BME680Sensor::updateState() { #ifdef FSCom + spiLock->lock(); bool update = false; if (stateUpdateCounter == 0) { /* First state update when IAQ accuracy is >= 3 */ @@ -127,6 +131,7 @@ void BME680Sensor::updateState() LOG_INFO("Can't write %s state (File: %s)", sensorName, bsecConfigFileName); } } + spiLock->unlock(); #else LOG_ERROR("ERROR: Filesystem not implemented"); #endif diff --git a/src/modules/Telemetry/Sensor/NAU7802Sensor.cpp b/src/modules/Telemetry/Sensor/NAU7802Sensor.cpp index 65f616686..1329c8d90 100644 --- a/src/modules/Telemetry/Sensor/NAU7802Sensor.cpp +++ b/src/modules/Telemetry/Sensor/NAU7802Sensor.cpp @@ -5,6 +5,7 @@ #include "../mesh/generated/meshtastic/telemetry.pb.h" #include "FSCommon.h" #include "NAU7802Sensor.h" +#include "SPILock.h" #include "SafeFile.h" #include "TelemetrySensor.h" #include @@ -111,13 +112,16 @@ bool NAU7802Sensor::saveCalibrationData() } else { okay = true; } + spiLock->lock(); okay &= file.close(); + spiLock->unlock(); return okay; } bool NAU7802Sensor::loadCalibrationData() { + spiLock->lock(); auto file = FSCom.open(nau7802ConfigFileName, FILE_O_READ); bool okay = false; if (file) { @@ -134,7 +138,8 @@ bool NAU7802Sensor::loadCalibrationData() } else { LOG_INFO("No %s state found (File: %s)", sensorName, nau7802ConfigFileName); } + spiLock->unlock(); return okay; } -#endif +#endif \ No newline at end of file diff --git a/src/xmodem.cpp b/src/xmodem.cpp index bf25e2da7..1d8c77760 100644 --- a/src/xmodem.cpp +++ b/src/xmodem.cpp @@ -49,6 +49,7 @@ **********************************************************************************************************************/ #include "xmodem.h" +#include "SPILock.h" #ifdef FSCom @@ -119,8 +120,11 @@ void XModemAdapter::handlePacket(meshtastic_XModem xmodemPacket) if ((xmodemPacket.seq == 0) && !isReceiving && !isTransmitting) { // NULL packet has the destination filename memcpy(filename, &xmodemPacket.buffer.bytes, xmodemPacket.buffer.size); + if (xmodemPacket.control == meshtastic_XModem_Control_SOH) { // Receive this file and put to Flash + spiLock->lock(); file = FSCom.open(filename, FILE_O_WRITE); + spiLock->unlock(); if (file) { sendControl(meshtastic_XModem_Control_ACK); isReceiving = true; @@ -132,14 +136,18 @@ void XModemAdapter::handlePacket(meshtastic_XModem xmodemPacket) break; } else { // Transmit this file from Flash LOG_INFO("XModem: Transmit file %s", filename); + spiLock->lock(); file = FSCom.open(filename, FILE_O_READ); + spiLock->unlock(); if (file) { packetno = 1; isTransmitting = true; xmodemStore = meshtastic_XModem_init_zero; xmodemStore.control = meshtastic_XModem_Control_SOH; xmodemStore.seq = packetno; + spiLock->lock(); xmodemStore.buffer.size = file.read(xmodemStore.buffer.bytes, sizeof(meshtastic_XModem_buffer_t::bytes)); + spiLock->unlock(); xmodemStore.crc16 = crc16_ccitt(xmodemStore.buffer.bytes, xmodemStore.buffer.size); LOG_DEBUG("XModem: STX Notify Send packet %d, %d Bytes", packetno, xmodemStore.buffer.size); if (xmodemStore.buffer.size < sizeof(meshtastic_XModem_buffer_t::bytes)) { @@ -159,7 +167,9 @@ void XModemAdapter::handlePacket(meshtastic_XModem xmodemPacket) if ((xmodemPacket.seq == packetno) && check(xmodemPacket.buffer.bytes, xmodemPacket.buffer.size, xmodemPacket.crc16)) { // valid packet + spiLock->lock(); file.write(xmodemPacket.buffer.bytes, xmodemPacket.buffer.size); + spiLock->unlock(); sendControl(meshtastic_XModem_Control_ACK); packetno++; break; @@ -178,16 +188,21 @@ void XModemAdapter::handlePacket(meshtastic_XModem xmodemPacket) case meshtastic_XModem_Control_EOT: // End of transmission sendControl(meshtastic_XModem_Control_ACK); + spiLock->lock(); file.flush(); file.close(); + spiLock->unlock(); isReceiving = false; break; case meshtastic_XModem_Control_CAN: // Cancel transmission and remove file sendControl(meshtastic_XModem_Control_ACK); + spiLock->lock(); file.flush(); file.close(); + FSCom.remove(filename); + spiLock->unlock(); isReceiving = false; break; case meshtastic_XModem_Control_ACK: @@ -195,7 +210,9 @@ void XModemAdapter::handlePacket(meshtastic_XModem xmodemPacket) if (isTransmitting) { if (isEOT) { sendControl(meshtastic_XModem_Control_EOT); + spiLock->lock(); file.close(); + spiLock->unlock(); LOG_INFO("XModem: Finished send file %s", filename); isTransmitting = false; isEOT = false; @@ -206,7 +223,9 @@ void XModemAdapter::handlePacket(meshtastic_XModem xmodemPacket) xmodemStore = meshtastic_XModem_init_zero; xmodemStore.control = meshtastic_XModem_Control_SOH; xmodemStore.seq = packetno; + spiLock->lock(); xmodemStore.buffer.size = file.read(xmodemStore.buffer.bytes, sizeof(meshtastic_XModem_buffer_t::bytes)); + spiLock->unlock(); xmodemStore.crc16 = crc16_ccitt(xmodemStore.buffer.bytes, xmodemStore.buffer.size); LOG_DEBUG("XModem: ACK Notify Send packet %d, %d Bytes", packetno, xmodemStore.buffer.size); if (xmodemStore.buffer.size < sizeof(meshtastic_XModem_buffer_t::bytes)) { @@ -224,7 +243,9 @@ void XModemAdapter::handlePacket(meshtastic_XModem xmodemPacket) if (isTransmitting) { if (--retrans <= 0) { sendControl(meshtastic_XModem_Control_CAN); + spiLock->lock(); file.close(); + spiLock->unlock(); LOG_INFO("XModem: Retransmit timeout, cancel file %s", filename); isTransmitting = false; break; @@ -232,8 +253,11 @@ void XModemAdapter::handlePacket(meshtastic_XModem xmodemPacket) xmodemStore = meshtastic_XModem_init_zero; xmodemStore.control = meshtastic_XModem_Control_SOH; xmodemStore.seq = packetno; + spiLock->lock(); file.seek((packetno - 1) * sizeof(meshtastic_XModem_buffer_t::bytes)); + xmodemStore.buffer.size = file.read(xmodemStore.buffer.bytes, sizeof(meshtastic_XModem_buffer_t::bytes)); + spiLock->unlock(); xmodemStore.crc16 = crc16_ccitt(xmodemStore.buffer.bytes, xmodemStore.buffer.size); LOG_DEBUG("XModem: NAK Notify Send packet %d, %d Bytes", packetno, xmodemStore.buffer.size); if (xmodemStore.buffer.size < sizeof(meshtastic_XModem_buffer_t::bytes)) { diff --git a/variants/mesh-tab/platformio.ini b/variants/mesh-tab/platformio.ini index 26b072cde..a1007a7f4 100644 --- a/variants/mesh-tab/platformio.ini +++ b/variants/mesh-tab/platformio.ini @@ -178,7 +178,7 @@ build_flags = ${mesh_tab_base.build_flags} -D LGFX_TOUCH_Y_MIN=0 -D LGFX_TOUCH_Y_MAX=479 -D LGFX_TOUCH_ROTATION=0 - -D LGFX_TOUCH_I2C_FREQ=1000000 + -D LGFX_TOUCH_I2C_FREQ=400000 ; 3.5" IPS TFT ILI9488 / FT6236: https://vi.aliexpress.com/item/1005006893699919.html [env:mesh-tab-3-5-IPS-capacitive] @@ -204,7 +204,7 @@ build_flags = ${mesh_tab_base.build_flags} -D LGFX_TOUCH_Y_MIN=0 -D LGFX_TOUCH_Y_MAX=479 -D LGFX_TOUCH_ROTATION=1 - -D LGFX_TOUCH_I2C_FREQ=1000000 + -D LGFX_TOUCH_I2C_FREQ=400000 ; 4.0" IPS TFT ILI9488 / FT6236: https://vi.aliexpress.com/item/1005007082906950.html [env:mesh-tab-4-0-IPS-capacitive] @@ -230,4 +230,4 @@ build_flags = ${mesh_tab_base.build_flags} -D LGFX_TOUCH_Y_MIN=0 -D LGFX_TOUCH_Y_MAX=479 -D LGFX_TOUCH_ROTATION=1 - -D LGFX_TOUCH_I2C_FREQ=1000000 \ No newline at end of file + -D LGFX_TOUCH_I2C_FREQ=400000 \ No newline at end of file