Fix G27 shifter buttons and add SPI3 support for wheel rim

Firmware/FFBoard/UserExtensions/Inc/SPIButtons.h

@@ -35,7 +35,7 @@ struct ButtonSourceConfig{
 class SPI_Buttons: public ButtonSource,public CommandHandler,public SPIDevice {
 
 	enum class SPIButtons_commands : uint32_t {
-		mode,btncut,btnpol,btnnum,cs,spispeed
+		mode,btncut,btnpol,btnnum,cs,spispeed,debug,syncread
 	};
 
 public:
@@ -45,13 +45,14 @@ class SPI_Buttons: public ButtonSource,public CommandHandler,public SPIDevice {
 	virtual ~SPI_Buttons();
 
 	uint8_t readButtons(uint64_t* buf);
+	uint16_t getBtnNum() override; // Override to return conf.numButtons
 
 	CommandStatus command(const ParsedCommand& cmd,std::vector<CommandReply>& replies) override;
 	void registerCommands();
 	virtual std::string getHelpstring(){return "SPI 2 Button";}
 
 	void saveFlash();
-	void restoreFlash();
+	virtual void restoreFlash();
 
 	const uint8_t maxButtons = 64;
 	std::string printModes(const std::vector<std::string>& names);
@@ -63,32 +64,29 @@ class SPI_Buttons: public ButtonSource,public CommandHandler,public SPIDevice {
 	void setSpiSpeed(uint8_t speedPreset);
 
 protected:
-	SPI_Buttons(uint16_t configuration_address, uint16_t configuration_address_2);
-
-private:
+	SPI_Buttons(uint16_t configuration_address, uint16_t configuration_address_2, SPIPort* spiPort = &external_spi, uint8_t instance = 0);
 	uint16_t configuration_address;
 	uint16_t configuration_address_2;
-	bool ready = false;
 	void setConfig(ButtonSourceConfig config);
 	virtual ButtonSourceConfig* getConfig();
-	void process(uint64_t* buf);
-	uint8_t bytes = 4;
+
+private:
+	bool ready = false;
 	uint64_t mask = 0xff;
 	uint8_t offset = 0;
 
-	ButtonSourceConfig conf;
+	static constexpr std::array<uint32_t,3> speedPresets= {SPI_BAUDRATEPRESCALER_16,SPI_BAUDRATEPRESCALER_32,SPI_BAUDRATEPRESCALER_64};
 
+protected:
+	void process(uint64_t* buf);
+	uint8_t bytes = 4;
 	uint8_t spi_buf[4] = {0};
-
-	static constexpr std::array<uint32_t,3> speedPresets= {SPI_BAUDRATEPRESCALER_16,SPI_BAUDRATEPRESCALER_32,SPI_BAUDRATEPRESCALER_64};
+	ButtonSourceConfig conf;
 };
 
 class SPI_Buttons_1 : public SPI_Buttons {
 public:
-	SPI_Buttons_1()
-		: SPI_Buttons{ADR_SPI_BTN_1_CONF, ADR_SPI_BTN_1_CONF_2} {
-			setInstance(0);
-		}
+	SPI_Buttons_1();
 
 	const ClassIdentifier getInfo() override;
 	static ClassIdentifier info;
@@ -98,10 +96,7 @@ class SPI_Buttons_1 : public SPI_Buttons {
 
 class SPI_Buttons_2 : public SPI_Buttons {
 public:
-	SPI_Buttons_2()
-		: SPI_Buttons{ADR_SPI_BTN_2_CONF, ADR_SPI_BTN_2_CONF_2} {
-			setInstance(1);
-		}
+	SPI_Buttons_2();
 
 	const ClassIdentifier getInfo() override;
 	static ClassIdentifier info;

Firmware/FFBoard/UserExtensions/Inc/ShifterAnalog.h

@@ -63,11 +63,14 @@ enum class ShifterAnalog_commands : uint32_t{
 		G27ShifterButtonClient(OutputPin& csPin);
 
 		static constexpr int numUserButtons{12};
+		static constexpr int bytesToRead{2}; // 16 buttons = 2 bytes (same as SPI Buttons with 16 buttons)
 
 		uint16_t getUserButtons();
 		bool getReverseButton();
-	private:
-		uint16_t buttonStates{0};
+		void startRead(); // Start a new DMA read (like SPI_Buttons does)
+		void spiRxCompleted(SPIPort* port) override; // Called when DMA completes
+
+		uint8_t spi_buf[2]{0}; // Buffer for SPI read (2 bytes for 16 buttons) - DMA writes directly here
 
 	};
 

Firmware/FFBoard/UserExtensions/Inc/eeprom_addresses.h

@@ -1,13 +1,13 @@
-/*		
- * eeprom_addresses.h		
- *		
- *  Created on: 24.01.2020		
- *  Author: Yannick		
- *		
- *	/!\ Generated from the file memory_map.csv 
-   / ! \ DO NOT EDIT THIS DIRECTLY !!!	
- */			
-		
+/*
+ * eeprom_addresses.h
+ *
+ *  Created on: 24.01.2020
+ *  Author: Yannick
+ *
+ *	/!\ Generated from the file memory_map.csv
+   / ! \ DO NOT EDIT THIS DIRECTLY !!!
+ */
+
 #ifndef EEPROM_ADDRESSES_H_
 #define EEPROM_ADDRESSES_H_
 
@@ -21,19 +21,19 @@ extern const uint16_t VirtAddVarTab[NB_OF_VAR];
 extern const uint16_t exportableFlashAddresses[NB_EXPORTABLE_ADR];
 
 
-		
-/* Add your addresses here. 0xffff is invalid as it marks an erased field.		
-Anything below 0x00ff is reserved for system variables.		
-		
-Use ranges that are clear to distinguish between configurations. Address ranges can have gaps.		
-Label the names clearly.		
-Example: 0x0100 - 0x01ff for one class and 0x0200-0x02ff for another class would be reasonable even if they each need only 3 variables		
-		
-		
-Important: Add your variable to the VirtAddVarTab[NB_OF_VAR] array in eeprom_addresses.c!		
-		
-Tip to check if a cell is intialized:		
-uint16_t EE_ReadVariable(uint16_t VirtAddress, uint16_t* Data) will return 1 if the address is not found or 0 if it was found.		
+
+/* Add your addresses here. 0xffff is invalid as it marks an erased field.
+Anything below 0x00ff is reserved for system variables.
+
+Use ranges that are clear to distinguish between configurations. Address ranges can have gaps.
+Label the names clearly.
+Example: 0x0100 - 0x01ff for one class and 0x0200-0x02ff for another class would be reasonable even if they each need only 3 variables
+
+
+Important: Add your variable to the VirtAddVarTab[NB_OF_VAR] array in eeprom_addresses.c!
+
+Tip to check if a cell is intialized:
+uint16_t EE_ReadVariable(uint16_t VirtAddress, uint16_t* Data) will return 1 if the address is not found or 0 if it was found.
 */
 // System variables
 #define ADR_HW_VERSION 1

Firmware/FFBoard/UserExtensions/Src/SPIButtons.cpp

@@ -30,6 +30,11 @@ bool SPI_Buttons_1::isCreatable(){
 	return (external_spi.hasFreePins());
 }
 
+SPI_Buttons_1::SPI_Buttons_1()
+	: SPI_Buttons{ADR_SPI_BTN_1_CONF, ADR_SPI_BTN_1_CONF_2, &external_spi, 0} {
+	restoreFlash(); // Call base class version (SPI_Buttons_1 doesn't override)
+}
+
 ClassIdentifier SPI_Buttons_2::info = {
 		 .name = "SPI Buttons 2" ,
 		 .id=CLSID_BTN_SPI,
@@ -42,14 +47,24 @@ bool SPI_Buttons_2::isCreatable(){
 	return false;//(external_spi.hasFreePins();
 }
 
+SPI_Buttons_2::SPI_Buttons_2()
+	: SPI_Buttons{ADR_SPI_BTN_2_CONF, ADR_SPI_BTN_2_CONF_2, &external_spi, 1} {
+	restoreFlash(); // Call base class version (SPI_Buttons_2 doesn't override)
+}
+
 
 // TODO check if pin is free
-SPI_Buttons::SPI_Buttons(uint16_t configuration_address, uint16_t configuration_address_2)
-	: CommandHandler("spibtn",CLSID_BTN_SPI,0), SPIDevice(external_spi,external_spi.getFreeCsPins()[0]){
+SPI_Buttons::SPI_Buttons(uint16_t configuration_address, uint16_t configuration_address_2, SPIPort* spiPort, uint8_t instance)
+	: CommandHandler("spibtn",CLSID_BTN_SPI,instance), SPIDevice(*spiPort,spiPort->getFreeCsPins()[0]){
 
 	this->configuration_address = configuration_address;
 	this->configuration_address_2 = configuration_address_2;
-	restoreFlash();
+	// NOTE: restoreFlash() is NOT called here - it must be called by derived class constructors
+	// This is because calling virtual functions in base class constructor doesn't dispatch to derived class
+
+	// Initialize with default settings - will be overwritten by restoreFlash() in derived class
+	this->spiConfig.peripheral.BaudRatePrescaler = speedPresets[1]; // Medium speed default
+	this->spiConfig.peripheral.FirstBit = SPI_FIRSTBIT_LSB;
 
 	registerCommands();
 	this->setCommandsEnabled(true);
@@ -77,6 +92,8 @@ void SPI_Buttons::registerCommands(){
 	registerCommand("btnnum", SPIButtons_commands::btnnum, "Number of buttons",CMDFLAG_GET | CMDFLAG_SET);
 	registerCommand("cs", SPIButtons_commands::cs, "SPI CS pin",CMDFLAG_GET | CMDFLAG_SET);
 	registerCommand("spispeed", SPIButtons_commands::spispeed, "SPI speed preset",CMDFLAG_INFOSTRING | CMDFLAG_GET | CMDFLAG_SET);
+	registerCommand("debug", SPIButtons_commands::debug, "Debug raw SPI data",CMDFLAG_GET);
+	registerCommand("syncread", SPIButtons_commands::syncread, "Test synchronous read",CMDFLAG_GET);
 }
 
 /**
@@ -95,31 +112,41 @@ void SPI_Buttons::setConfig(ButtonSourceConfig config){
 	OutputPin* newPin = spiPort.getCsPin(config.cs_num-1); // TODO update internal pin number if requested pin is blocked
 	if(newPin != nullptr){
 		this->spiConfig.cs = *newPin;
-
+		spiPort.reserveCsPin(this->spiConfig.cs);
+	}else{
+		// CS pin not found - this is an error condition
+		// Try to use first free pin as fallback
+		auto& freePins = spiPort.getFreeCsPins();
+		if(!freePins.empty()){
+			this->spiConfig.cs = freePins[0];
+			spiPort.reserveCsPin(this->spiConfig.cs);
+		}
 	}
-	spiPort.reserveCsPin(this->spiConfig.cs);
 	// Setup presets
 	if(conf.mode == SPI_BtnMode::TM){
 		this->spiConfig.cspol = true;
 		this->conf.cutRight = true;
-		this->spiConfig.peripheral.FirstBit = SPI_FIRSTBIT_LSB;
-		this->spiConfig.peripheral.CLKPhase = SPI_PHASE_1EDGE;
 		this->spiConfig.peripheral.CLKPolarity = SPI_POLARITY_LOW;
+		this->spiConfig.peripheral.CLKPhase = SPI_PHASE_1EDGE;
+
 	}else if(conf.mode == SPI_BtnMode::PISOSR){
 		this->spiConfig.cspol = false;
 		this->conf.cutRight = false;
-		this->spiConfig.peripheral.FirstBit = SPI_FIRSTBIT_LSB;
 		this->spiConfig.peripheral.CLKPhase = SPI_PHASE_2EDGE;
 		this->spiConfig.peripheral.CLKPolarity = SPI_POLARITY_HIGH; // its actually shifting on the rising edge but 165 will have the first output set even before clocking. First clock cycle is actually second bit so we sample at the falling edge and skip the first bit with that.
 	}
-	this->spiConfig.peripheral.BaudRatePrescaler = speedPresets[this->conf.spi_speed];
+//	spiPort.takeSemaphore();
+//	spiPort.configurePort(&this->spiConfig.peripheral);
+//	spiPort.giveSemaphore();
 	initSPI();
 	if(config.numButtons == 64){ // Special case
 			mask = 0xffffffffffffffff;
 	}else{
 		mask = (uint64_t)pow<uint64_t>(2,config.numButtons)-(uint64_t)1; // Must be done completely in 64 bit!
 	}
-	offset = 8 - (config.numButtons % 8);
+	// Calculate offset: if numButtons is multiple of 8, offset is 0
+	// Otherwise, offset is 8 - (numButtons % 8)
+	offset = (config.numButtons % 8 == 0) ? 0 : (8 - (config.numButtons % 8));
 
 	// Thrustmaster uses extra bits for IDs
 	if(config.mode == SPI_BtnMode::TM){
@@ -128,13 +155,20 @@ void SPI_Buttons::setConfig(ButtonSourceConfig config){
 		bytes = 1+((config.numButtons-1)/8);
 	}
 
+	// Update ButtonSource::btnnum so getBtnNum() returns correct value
 	this->btnnum = config.numButtons;
 }
 
 ButtonSourceConfig* SPI_Buttons::getConfig(){
 	return &this->conf;
 }
 
+uint16_t SPI_Buttons::getBtnNum(){
+	// Always return conf.numButtons as the source of truth
+	// btnnum inheritance issue causes it to not update correctly
+	return this->conf.numButtons;
+}
+
 void SPI_Buttons::setSpiSpeed(uint8_t speedPreset){
 	speedPreset = clip<uint8_t,uint8_t>(speedPreset,0,this->speedPresets.size());
 	this->conf.spi_speed = speedPreset;
@@ -175,12 +209,12 @@ uint8_t SPI_Buttons::readButtons(uint64_t* buf){
 	process(buf); // give back last buffer
 
 	if(spiPort.isTaken() || !ready)
-		return this->btnnum;	// Don't wait.
+		return this->conf.numButtons;	// Return conf.numButtons instead of btnnum
 
 	// CS pin and semaphore managed by spi port
 	spiPort.receive_DMA(spi_buf, bytes, this);
 
-	return this->btnnum;
+	return this->conf.numButtons;	// Return conf.numButtons instead of btnnum
 }
 
 std::string SPI_Buttons::printModes(const std::vector<std::string>& names){
@@ -199,6 +233,7 @@ CommandStatus SPI_Buttons::command(const ParsedCommand& cmd,std::vector<CommandR
 			ButtonSourceConfig* c = this->getConfig();
 			c->numButtons = cmd.val;
 			this->setConfig(*c);
+			this->saveFlash(); // Save to flash immediately
 		}else if(cmd.type == CMDtype::get){
 			replies.emplace_back(this->getBtnNum());
 		}else{
@@ -230,6 +265,7 @@ CommandStatus SPI_Buttons::command(const ParsedCommand& cmd,std::vector<CommandR
 	case SPIButtons_commands::mode:
 		if(cmd.type == CMDtype::set){
 			setMode((SPI_BtnMode)cmd.val);
+			this->saveFlash(); // Save to flash immediately
 		}else if(cmd.type == CMDtype::get){
 			replies.emplace_back((uint8_t)this->conf.mode);
 		}else if(cmd.type == CMDtype::info){
@@ -242,6 +278,7 @@ CommandStatus SPI_Buttons::command(const ParsedCommand& cmd,std::vector<CommandR
 	case SPIButtons_commands::spispeed:
 		if(cmd.type == CMDtype::set){
 			setSpiSpeed(cmd.val);
+			this->saveFlash(); // Save to flash immediately
 		}else if(cmd.type == CMDtype::get){
 			replies.emplace_back((uint8_t)this->conf.spi_speed);
 		}else if(cmd.type == CMDtype::info){
@@ -254,6 +291,59 @@ CommandStatus SPI_Buttons::command(const ParsedCommand& cmd,std::vector<CommandR
 	case SPIButtons_commands::cs:
 		if (handleGetSet(cmd, replies, this->conf.cs_num) == CommandStatus::OK ) {
 			setConfig(this->conf);
+			this->saveFlash(); // Save to flash immediately
+		}
+		break;
+
+	case SPIButtons_commands::debug:
+		if(cmd.type == CMDtype::get){
+			// Return raw SPI buffer data in hex format for debugging
+			std::string debug_data = "Raw:";
+			for(uint8_t i = 0; i < this->bytes; i++){
+				char hex[4];
+				sprintf(hex, "%02X", this->spi_buf[i]);
+				debug_data += hex;
+				if(i < this->bytes - 1) debug_data += " ";
+			}
+			debug_data += " offset:" + std::to_string(this->offset);
+			debug_data += " mask:0x" + std::to_string(this->mask);
+			debug_data += " inv:" + std::to_string(this->conf.invert);
+			debug_data += " cut:" + std::to_string(this->conf.cutRight);
+			replies.emplace_back(debug_data);
+		}else{
+			return CommandStatus::ERR;
+		}
+		break;
+
+	case SPIButtons_commands::syncread:
+		if(cmd.type == CMDtype::get){
+			// Detailed diagnostic of SPI communication
+			std::string result = "";
+
+			// Show which SPI port is being used
+			SPI_HandleTypeDef* hspi = spiPort.getPortHandle();
+			if(hspi->Instance == SPI2) result += "SPI2 ";
+			else if(hspi->Instance == SPI3) result += "SPI3 ";
+			else result += "SPI? ";
+
+			// Show CS pin info
+			result += "CS:" + std::to_string(this->conf.cs_num) + " ";
+
+			// Force a synchronous read
+			uint8_t test_buf[4] = {0xAA, 0xAA, 0xAA, 0xAA}; // Pre-fill with known pattern
+			HAL_StatusTypeDef status = HAL_SPI_Receive(hspi, test_buf, this->bytes, 100);
+
+			result += "HAL:" + std::to_string(status) + " ";
+			result += "Data:";
+			for(uint8_t i = 0; i < this->bytes; i++){
+				char hex[4];
+				sprintf(hex, "%02X", test_buf[i]);
+				result += hex;
+				if(i < this->bytes - 1) result += " ";
+			}
+			replies.emplace_back(result);
+		}else{
+			return CommandStatus::ERR;
 		}
 		break;