Merge pull request #104 from dzid26/fix_E07

Cut PWM above speed limit (fixes E09)

src/common.c

@@ -10,45 +10,57 @@
 #include "stm8s.h"
 #include "common.h"
 
-int16_t map_ui16(int16_t x, int16_t in_min, int16_t in_max, int16_t out_min, int16_t out_max) {
-    // if input min is smaller than output min, return the output min value
-    if (x < in_min) {
-        return out_min;
-    }
-
-    // if input max is bigger than output max, return the output max value
-    else if (x > in_max) {
-        return out_max;
-    }
 
-    // map the input to the output range, round up if mapping bigger ranges to smaller ranges
-    else if ((in_max - in_min) > (out_max - out_min)) {
-        return (int16_t)(((int32_t)(x - in_min) * (out_max - out_min + 1)) / (in_max - in_min + 1)) + out_min;
-    }
-
-    // map the input to the output range, round down if mapping smaller ranges to bigger ranges
-    else {
-        return (int16_t)(((int32_t)(x - in_min) * (out_max - out_min)) / (in_max - in_min)) + out_min;
+// Function to map a value from one range to another based on given input and output ranges.
+// Uses nearest integer rounding for precision.
+// Note: Input min has to be smaller than input max.
+// Parameters:
+// - in: Value to be mapped.
+// - in_min: Minimum value of the input range. 
+// - in_max: Maximum value of the input range.
+// - out_min: Minimum value of the output range.
+// - out_max: Maximum value of the output range.
+// Returns the mapped value within the specified output range.
+uint16_t map_ui16(uint16_t in, uint16_t in_min, uint16_t in_max, uint16_t out_min, uint16_t out_max) {
+    // If input is out of bounds, clamp it to the nearest boundary value
+    if (in < in_min) {return out_min;}
+    if (in >= in_max) {return out_max;}
+
+    // Calculate the input and output ranges
+    uint16_t in_range = in_max - in_min;
+
+    uint16_t out;
+    if (out_max < out_min) {
+        out = out_min - (uint16_t)(uint32_t)(((uint32_t)((uint32_t)(uint16_t)(in - in_min) * (uint32_t)(uint16_t)(out_min - out_max)) + (uint32_t)(uint16_t)(in_range/2U)) / in_range);
+    } else {
+        out = out_min + (uint16_t)(uint32_t)(((uint32_t)((uint32_t)(uint16_t)(in - in_min) * (uint32_t)(uint16_t)(out_max - out_min)) + (uint32_t)(uint16_t)(in_range/2U)) / in_range);
     }
+    return out;
 }
 
-uint8_t map_ui8(uint8_t x, uint8_t in_min, uint8_t in_max, uint8_t out_min, uint8_t out_max) {
-    // if input min is smaller than output min, return the output min value
-    if (x <= in_min) {
-        return out_min;
-    }
-
-    // if input max is bigger than output max, return the output max value
-    if (x >= in_max) {
-        return out_max;
+// Function to map 8bit a values from one range to another based on given input and output ranges.
+// Uses floor integer rounding for maximum performance.
+// Note: Input min has to be smaller than input max.
+// Parameters:
+// - in: Value to be mapped.
+// - in_min: Minimum value of the input range.
+// - in_max: Maximum value of the input range.
+// - out_min: Minimum value of the output range.
+// - out_max: Maximum value of the output range.
+// Returns the mapped value within the specified output range.
+uint8_t map_ui8(uint8_t in, uint8_t in_min, uint8_t in_max, uint8_t out_min, uint8_t out_max) {
+    // If input is out of bounds, clamp it to the nearest boundary value
+    if (in < in_min) {return out_min;}
+    if (in >= in_max) {return out_max;}
+
+    if (out_max < out_min) {
+        return out_min - (uint8_t)(uint16_t)((uint16_t)((uint8_t)(in - in_min) * (uint8_t)(out_min - out_max)) / (uint8_t)(in_max - in_min)); // cppcheck-suppress misra-c2012-10.8 ; direct cast to a wider essential to ensure mul in,a usage
+    } else {
+        return out_min + (uint8_t)(uint16_t)((uint16_t)((uint8_t)(in - in_min) * (uint8_t)(out_max - out_min)) / (uint8_t)(in_max - in_min)); // cppcheck-suppress misra-c2012-10.8 ; direct cast to a wider essential to ensure mul in,a usage
     }
-
-    if (out_max < out_min)
-        return (uint16_t)out_min - (uint16_t)((uint8_t)(x - in_min) * (uint8_t)(out_min - out_max)) / (uint8_t)(in_max - in_min);
-    else
-        return (uint16_t)out_min + (uint16_t)((uint8_t)(x - in_min) * (uint8_t)(out_max - out_min)) / (uint8_t)(in_max - in_min);
 }
 
+
 uint8_t ui8_min(uint8_t value_a, uint8_t value_b) {
     if (value_a < value_b) {
         return value_a;

src/common.h

@@ -49,8 +49,8 @@
 //#define ADVANCED_MODE                             1
 //#define CALIBRATION_MODE                          2
 
-int16_t map_ui16(int16_t x, int16_t in_min, int16_t in_max, int16_t out_min, int16_t out_max);
-uint8_t map_ui8(uint8_t x, uint8_t in_min, uint8_t in_max, uint8_t out_max, uint8_t out_min);
+uint16_t map_ui16(uint16_t in, uint16_t in_min, uint16_t in_max, uint16_t out_min, uint16_t out_max);
+uint8_t map_ui8(uint8_t in, uint8_t in_min, uint8_t in_max, uint8_t out_min, uint8_t out_max);
 uint8_t ui8_max(uint8_t value_a, uint8_t value_b);
 uint8_t ui8_min(uint8_t value_a, uint8_t value_b);
 uint16_t filter(uint16_t ui16_new_value, uint16_t ui16_old_value, uint8_t ui8_alpha);

src/ebike_app.c

@@ -1444,15 +1444,20 @@ static void apply_temperature_limiting(void)
 }
 
 
-static void apply_speed_limit(void)
-{
-    if (m_configuration_variables.ui8_wheel_speed_max) {
-        // set battery current target
-        ui8_adc_battery_current_target = map_ui16((uint16_t) ui16_wheel_speed_x10,
-                (uint16_t) (((uint8_t)(m_configuration_variables.ui8_wheel_speed_max) * (uint8_t)10U) - (uint8_t)20U),
-                (uint16_t) (((uint8_t)(m_configuration_variables.ui8_wheel_speed_max) * (uint8_t)10U) + (uint8_t)20U),
+static void apply_speed_limit(void) {
+    if (m_configuration_variables.ui8_wheel_speed_max > 0U) {
+		uint16_t speed_limit_low  = (uint16_t)((uint8_t)(m_configuration_variables.ui8_wheel_speed_max - 2U) * (uint8_t)10U); // casting literal to uint8_t ensures usage of MUL X,A
+		uint16_t speed_limit_high = (uint16_t)((uint8_t)(m_configuration_variables.ui8_wheel_speed_max + 2U) * (uint8_t)10U);
+
+        ui8_adc_battery_current_target = (uint8_t)map_ui16(ui16_wheel_speed_x10,
+                speed_limit_low,
+                speed_limit_high,
                 ui8_adc_battery_current_target,
-                0);
+                0U);
+
+		if (ui16_wheel_speed_x10 > speed_limit_high) {
+			ui8_duty_cycle_target = 0;
+		}
     }
 }
 

tests/test_map.py

@@ -0,0 +1,75 @@
+import pytest
+from sim._tsdz2 import ffi, lib as ebike # module generated from c-code
+import numpy as np
+from hypothesis import given, assume, strategies as st
+
+
+@pytest.mark.parametrize(
+		"x,    in_min, in_max, out_min, out_max, expected", [
+		( 4,	0,		16,		16,		0,		12),
+		( 1,	0,		2,		3,		0,		1),
+		( 1,	0,		3,		0,		2,		1),
+		])
+def test_maps_simple(x, in_min, in_max, out_min, out_max, expected):
+	map_ui8_result = ebike.map_ui8(x, in_min, in_max, out_min, out_max)
+	map_ui16_result = ebike.map_ui16(x, in_min, in_max, out_min, out_max)
+	assert map_ui8_result == pytest.approx(expected, abs=1), f'Expected map_ui8_result {expected}, got {map_ui8_result}'
+	assert map_ui16_result == expected, f'Expected map_ui16_result {expected}, got {map_ui16_result}'
+
+
+# Parameterized test function with different ticks values
+@pytest.mark.parametrize("x", range(20, 45))
+def test_compare_ui8_ui16_map_input_smaller_than_output(x):
+	in_min = 23
+	in_max = 43
+	out_min = 5
+	out_max = 250
+	map_ui8_result = ebike.map_ui8(x, in_min, in_max, out_min, out_max)
+	map_ui16_result = ebike.map_ui16(x, in_min, in_max, out_min, out_max)
+	# ! map_ui8 has lower precision so allow for an error of 1
+	assert map_ui16_result == pytest.approx(map_ui8_result, abs=1), f'Expected map_ui8_result {map_ui8_result} == map_ui16_result {map_ui16_result}'
+
+@pytest.mark.parametrize("x", range(20, 90))
+def test_compare_ui8_ui16_map_input_greater_than_output(x):
+	in_min = 23
+	in_max = 87
+	out_min = 5
+	out_max = 50
+	map_ui8_result = ebike.map_ui8(x, in_min, in_max, out_min, out_max)
+	map_ui16_result = ebike.map_ui16(x, in_min, in_max, out_min, out_max)
+
+	# ! map_ui8 has lower precision so allow for an error of 1
+	assert map_ui16_result == pytest.approx(map_ui8_result, abs=1), f'Expected map_ui8_result {map_ui8_result} == map_ui16_result {map_ui16_result}'
+
+
+
+# Define the hypothesis test for map_ui8
+@given(
+	x=st.integers(min_value=0, max_value=65535),
+	in_min=st.integers(min_value=0, max_value=65535),
+	in_max=st.integers(min_value=0, max_value=65535),
+	out_min=st.integers(min_value=0, max_value=65535),
+	out_max=st.integers(min_value=0, max_value=65535))
+def test_maps_full_ranges(x, in_min, in_max, out_min, out_max):
+	assume(in_min <= in_max)
+
+	expected = np.interp(x, [in_min, in_max], [out_min, out_max])
+	# !test map_ui8 only for 8 bit ranges
+	if max(x, in_min, in_max, out_min, out_max) < 2^8:
+		map_ui8_result = ebike.map_ui8(x, in_min, in_max, out_min, out_max)
+		# ! map_ui8 lowest precision is 1
+		assert map_ui8_result == pytest.approx(expected, abs=1), \
+			f"map_ui8({x}, {in_min}, {in_max}, {out_min}, {out_max}) returned {map_ui8_result}, expected {expected}"
+	else:
+		print(f'x={x}, in_min={in_min}, in_max={in_max}, out_min={out_min}, out_max={out_max}')
+
+	map_ui16_result = ebike.map_ui16(x, in_min, in_max, out_min, out_max)
+	# ! map_ui16 lowest precision is 0.5 (thanks to nearest rounding)
+	assert map_ui16_result == pytest.approx(expected, abs=.5), \
+		f"map_ui16({x}, {in_min}, {in_max}, {out_min}, {out_max}) returned {map_ui16_result}, expected {expected}"
+
+
+
+# Run the tests
+if __name__ == '__main__':
+	pytest.main()

tests/test_speed_limit.py

@@ -0,0 +1,41 @@
+import pytest
+from sim._tsdz2 import ffi, lib as ebike # module generated from c-code
+import numpy as np
+
+BATTERY_CURRENT_PER_10_BIT_ADC_STEP_X100 = 16
+mA_to_ADC = 100/BATTERY_CURRENT_PER_10_BIT_ADC_STEP_X100 / 1000
+
+# Set up initial values before each test
+@pytest.fixture(autouse=True)
+def setup_ebike():
+    # Set up initial values before each test
+    ebike.m_configuration_variables.ui8_wheel_speed_max = 25
+    ebike.ui8_duty_cycle_target = 255 # set by assistance function
+    ebike.ui8_adc_battery_current_target = int(5000 * mA_to_ADC)  # 5000mA set by assistance function
+    yield
+    # Teardown after each test (optional)
+
+
+def apply_speed_limit_float(speed):
+    speed_max = ebike.m_configuration_variables.ui8_wheel_speed_max
+    speed_lo = speed_max - 2
+    speed_hi = speed_max + 2
+    curr_target = ebike.ui8_adc_battery_current_target
+    current_lim = np.interp(speed, [speed_lo, speed_hi], [curr_target, 0])
+    return current_lim
+
+# Parameterized test function with different ticks values
+@pytest.mark.parametrize("speed", [0, 22.9, 23, 23.5, 24, 24.5, 25, 25.5, 26, 26.5, 27, 27.1, 30])
+def test_apply_speed_limit(speed):
+    ebike.ui16_wheel_speed_x10 = int(speed * 10)
+
+    expected = apply_speed_limit_float(speed) # this has to run first
+    ebike.apply_speed_limit()
+    result = ebike.ui8_adc_battery_current_target
+
+    assert result ==pytest.approx(expected, rel=1e-1, abs=0.1), f'Expected target {expected/mA_to_ADC}mA, got {result/mA_to_ADC}mA'
+
+
+# Run the tests
+if __name__ == '__main__':
+    pytest.main()