test coverage and some magic constant refactoring

firmware/controllers/lua/can_filter.cpp

@@ -0,0 +1,41 @@
+#include "pch.h"
+#include "can_filter.h"
+
+static constexpr size_t maxFilterCount = 48;
+
+size_t filterCount = 0;
+CanFilter filters[maxFilterCount];
+
+CanFilter* getFilterForId(size_t busIndex, int Id) {
+	for (size_t i = 0; i < filterCount; i++) {
+		auto& filter = filters[i];
+
+		if (filter.accept(Id)) {
+			if (filter.Bus == ANY_BUS || filter.Bus == busIndex) {
+				return &filter;
+			}
+		}
+	}
+
+	return nullptr;
+}
+
+void resetLuaCanRx() {
+	// Clear all lua filters - reloading the script will reinit them
+	filterCount = 0;
+}
+
+void addLuaCanRxFilter(int32_t eid, uint32_t mask, int bus, int callback) {
+	if (filterCount >= maxFilterCount) {
+		firmwareError(OBD_PCM_Processor_Fault, "Too many Lua CAN RX filters");
+	}
+
+	efiPrintf("Added Lua CAN RX filter id 0x%x mask 0x%x with%s custom function", eid, mask, (callback == -1 ? "out" : ""));
+
+	filters[filterCount].Id = eid;
+	filters[filterCount].Mask = mask;
+	filters[filterCount].Bus = bus;
+	filters[filterCount].Callback = callback;
+
+	filterCount++;
+}

firmware/controllers/lua/can_filter.h

@@ -0,0 +1,25 @@
+// can_filter.h
+
+#pragma once
+
+#define FILTER_SPECIFIC 0x1FFFFFFF
+#define ANY_BUS -1
+
+struct CanFilter {
+	int32_t Id;
+	int32_t Mask;
+
+	int Bus;
+	int Callback;
+
+	bool accept(int Id) {
+	    return (Id & this->Mask) == this->Id;
+	}
+};
+
+// Called when the user script is unloaded, resets any CAN rx filters
+void resetLuaCanRx();
+// Adds a frame ID to listen to
+void addLuaCanRxFilter(int32_t eid, uint32_t mask, int bus, int callback);
+
+CanFilter* getFilterForId(size_t busIndex, int Id);

firmware/controllers/lua/lua.cpp

@@ -7,6 +7,7 @@
 
 #include "lua.hpp"
 #include "lua_hooks.h"
+#include "can_filter.h"
 
 #define TAG "LUA "
 

firmware/controllers/lua/lua.mk

@@ -3,6 +3,7 @@ LUA_EXT=$(PROJECT_DIR)/ext/lua
 
 ALLCPPSRC += $(LUA_DIR)/lua.cpp \
 			 $(LUA_DIR)/lua_hooks.cpp \
+			 $(LUA_DIR)/can_filter.cpp \
 			 $(LUA_DIR)/lua_hooks_util.cpp \
 			 $(LUA_DIR)/script_impl.cpp \
 			 $(LUA_DIR)/generated/output_lookup_generated.cpp \

firmware/controllers/lua/lua_can_rx.cpp

@@ -1,36 +1,12 @@
 #include "pch.h"
 
+#include "can_filter.h"
+
+
 #if EFI_CAN_SUPPORT
 
 #include "rusefi_lua.h"
 
-static constexpr size_t maxFilterCount = 48;
-
-struct Filter {
-	int32_t Id;
-	int32_t Mask;
-
-	int Bus;
-	int Callback;
-};
-
-size_t filterCount = 0;
-Filter filters[maxFilterCount];
-
-static Filter* getFilterForFrame(size_t busIndex, const CANRxFrame& frame) {
-	for (size_t i = 0; i < filterCount; i++) {
-		auto& filter = filters[i];
-
-		if ((CAN_ID(frame) & filter.Mask) == filter.Id) {
-			if (filter.Bus == -1 || filter.Bus == busIndex) {
-				return &filter;
-			}
-		}
-	}
-
-	return nullptr;
-}
-
 // Stores information about one received CAN frame: which bus, plus the actual frame
 struct CanFrameData {
 	uint8_t BusIndex;
@@ -46,7 +22,7 @@ chibios_rt::Mailbox<CanFrameData*, canFrameCount> freeBuffers;
 chibios_rt::Mailbox<CanFrameData*, canFrameCount> filledBuffers;
 
 void processLuaCan(const size_t busIndex, const CANRxFrame& frame) {
-	auto filter = getFilterForFrame(busIndex, frame);
+	auto filter = getFilterForId(busIndex, CAN_ID(frame));
 
 	// Filter the frame if we aren't listening for it
 	if (!filter) {
@@ -164,24 +140,4 @@ void initLuaCanRx() {
 	}
 }
 
-void resetLuaCanRx() {
-	// Clear all lua filters - reloading the script will reinit them
-	filterCount = 0;
-}
-
-void addLuaCanRxFilter(int32_t eid, uint32_t mask, int bus, int callback) {
-	if (filterCount >= maxFilterCount) {
-		firmwareError(OBD_PCM_Processor_Fault, "Too many Lua CAN RX filters");
-	}
-
-	efiPrintf("Added Lua CAN RX filter id 0x%x mask 0x%x with%s custom function", eid, mask, (callback == -1 ? "out" : ""));
-
-	filters[filterCount].Id = eid;
-	filters[filterCount].Mask = mask;
-	filters[filterCount].Bus = bus;
-	filters[filterCount].Callback = callback;
-
-	filterCount++;
-}
-
 #endif // EFI_CAN_SUPPORT

firmware/controllers/lua/lua_hooks.cpp

@@ -7,6 +7,7 @@
 #include "airmass.h"
 #include "lua_airmass.h"
 #include "value_lookup.h"
+#include "can_filter.h"
 #if EFI_CAN_SUPPORT || EFI_UNIT_TEST
 #include "can_msg_tx.h"
 #endif // EFI_CAN_SUPPORT
@@ -465,7 +466,7 @@ int lua_canRxAdd(lua_State* l) {
 	uint32_t eid;
 
 	// defaults if not passed
-	int bus = -1;
+	int bus = ANY_BUS;
 	int callback = -1;
 
 	switch (lua_gettop(l)) {
@@ -499,7 +500,7 @@ int lua_canRxAdd(lua_State* l) {
 			return luaL_error(l, "Wrong number of arguments to canRxAdd. Got %d, expected 1, 2, or 3.");
 	}
 
-	addLuaCanRxFilter(eid, 0x1FFFFFFF, bus, callback);
+	addLuaCanRxFilter(eid, FILTER_SPECIFIC, bus, callback);
 
 	return 0;
 }
@@ -509,7 +510,7 @@ int lua_canRxAddMask(lua_State* l) {
 	uint32_t mask;
 
 	// defaults if not passed
-	int bus = -1;
+	int bus = ANY_BUS;
 	int callback = -1;
 
 	switch (lua_gettop(l)) {

firmware/controllers/lua/rusefi_lua.h

@@ -58,10 +58,6 @@ void testLuaExecString(const char* script);
 
 // Lua CAN rx feature
 void initLuaCanRx();
-// Called when the user script is unloaded, resets any CAN rx filters
-void resetLuaCanRx();
-// Adds a frame ID to listen to
-void addLuaCanRxFilter(int32_t eid, uint32_t mask, int bus, int callback);
 
 // Called from the Lua loop to process any pending CAN frames
 void doLuaCanRx(LuaHandle& ls);

unit_tests/tests/tests.mk

@@ -37,6 +37,7 @@ TESTS_SRC_CPP = \
 	tests/lua/test_lua_vag.cpp \
 	tests/lua/test_lua_with_engine.cpp \
 	tests/lua/test_lua_hooks.cpp \
+	tests/lua/test_can_filter.cpp \
 	tests/sensor/test_cj125.cpp \
 	tests/test_change_engine_type.cpp \
 	tests/util/test_scaled_channel.cpp \