Merge pull request #133 from senseshift/feature/reduce-overhead

perf: reduce vtable usage

.github/scripts/compare-memory-usage.py

@@ -0,0 +1,87 @@
+import json
+import pandas as pd
+import argparse
+
+
+# Function to load JSON data from files
+def load_json(filename):
+    with open(filename, 'r') as f:
+        return json.load(f)
+
+
+# Function to compare values and create formatted strings
+def compare_values(base, current, key):
+    if key not in base and key not in current:
+        return None
+    if key not in base:
+        return f"🆕 {current[key]}"
+    if key not in current:
+        return f"❌"
+
+    base_value = base[key]
+    current_value = current[key]
+
+    if base_value == current_value:
+        return f"🆗 {base_value}"
+
+    change = current_value - base_value
+    sign = "+" if change > 0 else "-"
+    symbol = "⚠️" if change > 0 else "⬇️"
+
+    # round to 2 decimal places
+    change = round(abs(change), 2)
+    return f"{symbol} {current_value} <sub>{sign}{change}</sub>"
+
+
+# Function to generate Markdown table
+def generate_markdown_table(base_data, current_data):
+    table_data = []
+
+    all_keys = set(base_data.keys()).union(set(current_data.keys()))
+    all_keys = sorted(all_keys)
+
+    for key in all_keys:
+        base_ram = base_data.get(key, {}).get("ram", {})
+        current_ram = current_data.get(key, {}).get("ram", {})
+
+        base_flash = base_data.get(key, {}).get("flash", {})
+        current_flash = current_data.get(key, {}).get("flash", {})
+
+        row = [
+            key.replace("-usage", ""),
+            compare_values(base_ram, current_ram, "percentage"),
+            compare_values(base_ram, current_ram, "used"),
+            compare_values(base_flash, current_flash, "percentage"),
+            compare_values(base_flash, current_flash, "used")
+        ]
+        table_data.append(row)
+
+    df = pd.DataFrame(table_data, columns=["Variant", "RAM, %", "RAM, bytes", "Flash, %", "Flash, bytes"])
+
+    return df.to_markdown(index=False)
+
+
+# Main function to handle CLI arguments and execute the script
+def main():
+    parser = argparse.ArgumentParser(description='Compare JSON files and generate a markdown table.')
+    parser.add_argument('base', type=str, help='Path to the base JSON file')
+    parser.add_argument('current', type=str, help='Path to the current JSON file')
+    parser.add_argument('--output', type=str, help='Path to the output markdown file')
+    args = parser.parse_args()
+
+    base_data = load_json(args.base)
+    current_data = load_json(args.current)
+
+    markdown_table = generate_markdown_table(base_data, current_data)
+
+    if args.output:
+        with open(args.output, 'w') as f:
+            f.write(markdown_table)
+        print(f"Markdown table generated and saved to {args.output}")
+    else:
+        print(markdown_table)
+
+
+# usage: python compare-memory-usage.py base.json current.json --output table.md
+if __name__ == "__main__":
+    main()

.github/workflows/ci.yml

@@ -49,11 +49,11 @@ jobs:
             coverage: true
             battery_flag: SS_BATTERY_ENABLED=true
             nimble_flag: SS_USE_NIMBLE=true
-          # - target: bhaptics_tactsuit_x40
-          #   os: ubuntu-latest
-          #   coverage: true
-          #   battery_flag: SS_BATTERY_ENABLED=true
-          #   nimble_flag: SS_USE_NIMBLE=false
+          - target: bhaptics_tactsuit_x40
+            os: ubuntu-latest
+            coverage: true
+            battery_flag: SS_BATTERY_ENABLED=true
+            nimble_flag: SS_USE_NIMBLE=false
           - target: bhaptics_tactsuit_x40
             os: ubuntu-latest
             coverage: false
@@ -532,8 +532,6 @@ jobs:
           pattern: '*-usage'
           merge-multiple: true
 
-      - run: ls -lahR current-usage
-
       - name: Merge usage files
         shell: python
         run: |
@@ -579,6 +577,68 @@ jobs:
           name: merged-usage
           path: current-usage/merged.json
 
+      - name: Get base branch job ID
+        if: github.event_name == 'pull_request'
+        id: base_branch_workflow
+        uses: actions/github-script@v7
+        with:
+          script: |
+            const { data: workflows } = await github.rest.actions.listWorkflowRuns({
+              owner: context.repo.owner,
+              repo: context.repo.repo,
+              branch: context.payload.pull_request.base.ref,
+              event: 'push',
+              workflow_id: 'ci.yml',
+              status: 'completed',
+              per_page: 1
+            });
+
+            const baseBranchJobId = workflows.workflow_runs[0].id;
+            console.log(baseBranchJobId);
+
+            return baseBranchJobId;
+
+
+      - name: Download base branch usage.json artifact
+        if: github.event_name == 'pull_request'
+        id: download-base-usage
+        uses: actions/download-artifact@v4
+        with:
+          name: merged-usage
+          path: base-usage
+          github-token: ${{ secrets.GITHUB_TOKEN }}
+          repository: ${{ github.repository }}
+          run-id: ${{ steps.base_branch_workflow.outputs.result }}
+
+      - uses: actions/setup-python@v2
+        if: github.event_name == 'pull_request'
+        with:
+          python-version: '3.x'
+
+      - name: Report memory usage table
+        if: github.event_name == 'pull_request'
+        shell: bash
+        run: |
+          python -m pip install --upgrade pip pandas tabulate
+
+          cat ./base-usage/merged.json
+          cat ./current-usage/merged.json
+
+          echo "## Memory Usage Comparison" > memory-usage-comparison.md
+          echo "" >> memory-usage-comparison.md
+          echo "<details>" >> memory-usage-comparison.md
+          echo "  <summary>Click to expand</summary>" >> memory-usage-comparison.md
+          echo "" >> memory-usage-comparison.md
+          python ./.github/scripts/compare-memory-usage.py ./base-usage/merged.json ./current-usage/merged.json | tee -a memory-usage-comparison.md
+          echo "</details>" >> memory-usage-comparison.md
+
+          cat ./memory-usage-comparison.md > $GITHUB_STEP_SUMMARY
+
+      - uses: thollander/actions-comment-pull-request@v2
+        if: github.event_name == 'pull_request'
+        with:
+          filePath: memory-usage-comparison.md
+          comment_tag: memory-usage-comparison
   wokwi:
     needs:
       - build-bhaptics

firmware/mode_configs/bhaptics/tactsuit_x40.cpp

@@ -57,26 +57,24 @@ void setupMode()
 
     // Assign the pins on the configured PCA9685s and PWM pins to locations on the
     // vest
-    auto frontOutputs = PlaneMapper_Margin::
-      mapMatrixCoordinates<FloatPlane::Actuator*, std::array<std::array<FloatPlane::Actuator*, 4>, 5>>({ {
-        // clang-format off
+    auto frontOutputs = PlaneMapper_Margin::mapMatrixCoordinates<FloatPlane::Actuator*>({
+      // clang-format off
           { new PCA9685Output(pwm0, 0),  new PCA9685Output(pwm0, 1),  new PCA9685Output(pwm0, 2),  new PCA9685Output(pwm0, 3)  },
           { new PCA9685Output(pwm0, 4),  new PCA9685Output(pwm0, 5),  new PCA9685Output(pwm0, 6),  new PCA9685Output(pwm0, 7)  },
           { new PCA9685Output(pwm0, 8),  new PCA9685Output(pwm0, 9),  new PCA9685Output(pwm0, 10), new PCA9685Output(pwm0, 11) },
           { new PCA9685Output(pwm0, 12), new PCA9685Output(pwm0, 13), new PCA9685Output(pwm0, 14), new PCA9685Output(pwm0, 15) },
           { new LedcOutput(32),          new LedcOutput(33),          new LedcOutput(25),          new LedcOutput(26)          },
-        // clang-format on
-      } });
-    auto backOutputs = PlaneMapper_Margin::
-      mapMatrixCoordinates<FloatPlane::Actuator*, std::array<std::array<FloatPlane::Actuator*, 4>, 5>>({ {
-        // clang-format off
+      // clang-format on
+    });
+    auto backOutputs = PlaneMapper_Margin::mapMatrixCoordinates<FloatPlane::Actuator*>({
+      // clang-format off
           { new PCA9685Output(pwm1, 0),  new PCA9685Output(pwm1, 1),  new PCA9685Output(pwm1, 2),  new PCA9685Output(pwm1, 3)  },
           { new PCA9685Output(pwm1, 4),  new PCA9685Output(pwm1, 5),  new PCA9685Output(pwm1, 6),  new PCA9685Output(pwm1, 7)  },
           { new PCA9685Output(pwm1, 8),  new PCA9685Output(pwm1, 9),  new PCA9685Output(pwm1, 10), new PCA9685Output(pwm1, 11) },
           { new PCA9685Output(pwm1, 12), new PCA9685Output(pwm1, 13), new PCA9685Output(pwm1, 14), new PCA9685Output(pwm1, 15) },
           { new LedcOutput(27),          new LedcOutput(14),          new LedcOutput(12),          new LedcOutput(13)          },
-        // clang-format on
-      } });
+      // clang-format on
+    });
 
     app->getVibroBody()->addTarget(Target::ChestFront, new FloatPlane_Closest(frontOutputs));
     app->getVibroBody()->addTarget(Target::ChestBack, new FloatPlane_Closest(backOutputs));

include/senseshift.h

@@ -13,7 +13,7 @@ class Application final : public IEventDispatcher {
   public:
     Application();
 
-    [[nodiscard]] auto getVibroBody() const -> Body::Haptics::FloatBody*
+    auto getVibroBody() const -> Body::Haptics::FloatBody*
     {
         return this->vibro_body_;
     }

lib/arduino/senseshift/arduino/input/sensor/analog.hpp

@@ -2,7 +2,7 @@
 
 #include <cstdint>
 
-#include <senseshift/input/sensor/sensor.hpp>
+#include <senseshift/input/sensor.hpp>
 
 #include <Arduino.h>
 
@@ -33,7 +33,7 @@ class AnalogSimpleSensor : public ::SenseShift::Input::IFloatSimpleSensor {
         pinMode(this->pin_, INPUT);
     }
 
-    [[nodiscard]] inline auto getValue() -> float override
+    inline auto getValue() -> float override
     {
         const std::uint16_t raw = analogRead(this->pin_);
         return static_cast<float>(raw) / ANALOG_MAX;

lib/arduino/senseshift/arduino/input/sensor/digital.hpp

@@ -4,7 +4,7 @@
 
 #include <Arduino.h>
 
-#include <senseshift/input/sensor/sensor.hpp>
+#include <senseshift/input/sensor.hpp>
 
 namespace SenseShift::Arduino::Input {
 class DigitalSimpleSensor : public ::SenseShift::Input::IBinarySimpleSensor {
@@ -21,7 +21,7 @@ class DigitalSimpleSensor : public ::SenseShift::Input::IBinarySimpleSensor {
         pinMode(this->pin_, this->mode_);
     }
 
-    [[nodiscard]] auto getValue() -> bool override
+    auto getValue() -> bool override
     {
         return digitalRead(this->pin_) == this->inverted_;
     }

lib/arduino/senseshift/arduino/input/sensor/multiplexer.hpp

@@ -11,19 +11,19 @@
 #include "senseshift/arduino/input/sensor/digital.hpp"
 
 #include <senseshift/core/component.hpp>
-#include <senseshift/input/sensor/sensor.hpp>
+#include <senseshift/input/sensor.hpp>
 
 namespace SenseShift::Arduino::Input {
 template<size_t N>
-class Multiplexer : public IInitializable {
+class Multiplexer {
   public:
     /// \param pins The pins to use for the multiplexer.
     explicit Multiplexer(const std::array<std::uint8_t, N>& pins, const size_t switch_delay_us = 2) :
       pins_(pins), switch_delay_us_(switch_delay_us)
     {
     }
 
-    void init() override
+    void init()
     {
         for (const auto pin : this->pins_) {
             pinMode(pin, OUTPUT);
@@ -74,7 +74,7 @@ class MultiplexedAnalogSensor : public AnalogSimpleSensor {
         AnalogSimpleSensor::init();
     }
 
-    [[nodiscard]] auto getValue() -> float override
+    auto getValue() -> float override
     {
         this->component_->selectChannel(this->channel_);
 
@@ -86,17 +86,16 @@ class MultiplexedAnalogSensor : public AnalogSimpleSensor {
     const std::uint8_t channel_;
 };
 
+template<size_t N>
 class MultiplexedDigitalSensor : public DigitalSimpleSensor {
   public:
     /// \param component The CD74HC4057 Component to use.
     /// \param pin The SIG pin of the sensor.
     /// \param channel The channel to read from.
-    MultiplexedDigitalSensor(
-      MUX_CD74HC4057Component* component, const std::uint8_t pin_sig, const std::uint8_t channel
-    ) :
+    MultiplexedDigitalSensor(Multiplexer<N>* component, const std::uint8_t pin_sig, const std::uint8_t channel) :
       component_(component), DigitalSimpleSensor(pin_sig), channel_(channel)
     {
-        assert(channel < 16 && "Channel out of range");
+        assert(channel < (1 << N) && "Channel out of range");
     }
 
     void init() override
@@ -105,15 +104,15 @@ class MultiplexedDigitalSensor : public DigitalSimpleSensor {
         DigitalSimpleSensor::init();
     }
 
-    [[nodiscard]] auto getValue() -> bool override
+    auto getValue() -> bool override
     {
         this->component_->selectChannel(this->channel_);
 
         return DigitalSimpleSensor::getValue();
     }
 
   private:
-    MUX_CD74HC4057Component* component_;
+    Multiplexer<N>* component_;
     const std::uint8_t channel_;
 };
 

lib/arduino_esp32/senseshift/arduino/output/ledc.hpp

@@ -41,7 +41,7 @@ class LedcOutput : public ::SenseShift::Output::IFloatOutput {
         }
     }
 
-    [[nodiscard]] inline auto getMaxValue() const -> std::uint32_t
+    inline auto getMaxValue() const -> std::uint32_t
     {
         return (1 << this->analog_resolution_) - 1;
     }

lib/battery/senseshift/battery/input/battery_sensor.hpp

@@ -5,7 +5,7 @@
 #include "senseshift/battery/battery.hpp"
 
 #include <senseshift/core/helpers.hpp>
-#include <senseshift/input/sensor/sensor.hpp>
+#include <senseshift/input/sensor.hpp>
 
 namespace SenseShift::Battery::Input {
 /// Abstract battery sensor
@@ -44,7 +44,7 @@ class LookupTableInterpolateBatterySensor : public IBatterySensor {
     }
 
   protected:
-    [[nodiscard]] auto lookupInterpolateLevel(VoltageType voltage) -> float
+    auto lookupInterpolateLevel(VoltageType voltage) -> float
     {
         return ::SenseShift::lookup_table_interpolate_linear<Container, VoltageType, float>(
           *this->lookup_table_,