Feat(#1): DSP, Reconstruction Filter and Tmc init optimisation by manoukianv · Pull Request #167 · Ultrawipf/OpenFFBoard

manoukianv · 2025-11-09T20:33:42Z

fix: compilation issue

vospascal · 2026-05-28T08:29:15Z

+	float32_t spline_y[4] = {0}; 		// Buffer value
+	float32_t spline_y2[4] = {0};    	// Buffer for Spline Natural
+	float32_t spline_scratch[8] = {0}; 	// Buffer for Spline Natural
+	arm_spline_instance_f32 spline_instance; // Instance pour CMSIS-DSP


good idea to translate to english

vospascal · 2026-05-28T08:29:22Z

+	float spline_x[4] = {0}; 		// Buffer time(en us)
+	float spline_y[4] = {0}; 		// Buffer value
+#endif
+	bool isSplineReady = false;	// Le tampon est-il plein ?


good idea to translate to english

vospascal · 2026-05-28T08:31:04Z


-
+/**
+ * @brief Contient l'état et les tampons pour


good idea to translate to english

Ultrawipf · 2026-05-28T09:02:51Z


-	CommandStatus command(const ParsedCommand& cmd,std::vector<CommandReply>& replies);
+	/**
+     * @brief Met à jour les tampons d'interpolation pour un effet non-conditionnel.


Rewrite the following comments in english

Ultrawipf · 2026-05-28T09:07:59Z

+		// 3. Apply the reduction to the main torque.
+		// We must only reduce the magnitude of the torque, not invert it.
+		reductionAmount = clip(reductionAmount, 0.0f, fabsf((float)torque));
+		if(torque > 0) {


Maybe make one common implementation and only switch the actual PID functions to dsp. The code is mostly identical but structured slightly differently making it harder to maintain/read

I'm sorry I'm not sure to understand:
Does the idea is to make generic call and make functions that include the ifdef on dsp in those function? I can, I think you prefer see the real impact.

Ultrawipf · 2026-05-28T09:44:59Z

@@ -103,81 +107,100 @@
 */
 void EffectsCalculator::calculateEffects(std::vector<std::unique_ptr<Axis>> &axes)


Explain why the order was changed and what has been changed in this function compared to the previous implementation.

Ultrawipf · 2026-05-28T09:45:47Z

-	int32_t force = 0;
 	int axisCount = axes.size();
-	int32_t forces[MAX_AXIS] = {0};
+	int64_t forces[MAX_AXIS] = {0};


Why is this 64b now if the output forces are 16b? A 32b variable should be enough to prevent overflows when calculating in 16b.

This is an issue, my bad

Ultrawipf · 2026-05-28T10:00:21Z

-				rampupFactor = ( 1 + sin(phaseRad ) ) / 2;						// sin value is -1..1 range, we translate it to 0..2 and we scale it by 2
-
+#ifdef USE_DSP_FUNCTIONS
+				float phaseRad = PI * ((fabsf (speed) / speedRampupCeil) - 0.5f);// we start to compute the normalized angle (speed / normalizedSpeed@5%) and translate it of -1/2PI to translate sin on 1/2 periode


Code duplication of the phaseRad depending on dsp function definition. Just use M_PI and cast to float if required. It is the same constant. Same as above in the other PI/M_PI variations.

Yes I just prefer to use the dsp PI value instead of the constant to have the max precision that the dsp offers.
Not mandatory, but as PI is provided, I'm using it.
As you prefer

Ultrawipf · 2026-05-28T10:26:41Z

 		break;
 	}

 	case FFB_EFFECT_RAMP:


Ramp doesn't have a magnitude but still calls the interpolation?

you right, I add a selector to exclude hard computing for ramp.

Ultrawipf · 2026-05-28T11:37:51Z

 void Flash_Dump(std::vector<std::tuple<uint16_t,uint16_t>> *result,bool includeAll = false);
 bool Flash_Format();

+#ifdef COGGING_TABLE_FLASH_START_ADDRESS


Actual implementation not in this PR. Seems to be in #169

You right I failed the split, to much code to d split, at the beginning, I have only 1 commit.

Ultrawipf · 2026-05-28T11:43:17Z

+void EffectsCalculator::updateEffectReconstruction(FFB_Effect* effect, float new_magnitude, float new_offset, bool is_periodic)
+{
+	// Update target variables (for NONE mode and fallback)
+    effect->magnitude = (int16_t)new_magnitude;


why is this changing the effect magnitude again? Shouldn't it be set by the game?

It think the misunderstanding come from wrong code in wrong branch during the split again, I try to make it simpler but split is not really good.
This method is called by the ffb process to store magnitude and offset on effect compatible with the reconstruction filter. When we process the ffb from hid we need to do 2 things:

Store the magnitude and offset as previous in the code

Store the magnitude and offset in the "temporal structure" to be able to compute the slicing value.

The code you point is the storing of the value I move here, but we can also let in the ffb. I regroup all the process on magnitude and offset here.
Give me your preference.

You are absolutely right. The misunderstanding definitely came from me trying to regroup everything during the split, but it violates the separation of concerns.
HidFFB should be the one responsible for setting the base state ( effect->magnitude and effect->offset ) since it decodes the USB HID reports from the game. EffectsCalculator should only handle the DSP and filter logic. By mixing them, it obscured the flow of data.
I have reverted this: HidFFB now assigns the magnitude and offset directly just like it did before, and then it calls EffectsCalculator::updateEffectReconstruction(...) purely to push the new samples into the temporal structure for the reconstruction filter.

Ultrawipf · 2026-05-28T11:47:56Z


-	effect_p.magnitude = data->magnitude;
+	//effect_p.magnitude = data->magnitude;
+	effects_calc->updateEffectReconstruction(&effect_p, (float)data->magnitude, 0.0f, false);


Does this additional call break other FFB sources like SerialFFB.cpp?

I don't see the SerialFFB 😥
So I don't apply reconstruction filter on them, so yes I make a break :-/

Ultrawipf · 2026-05-28T11:53:25Z

 	if(!overridesCondition){
-		float phaseX = M_PI*2.0 * (effect->directionX/36000.0f);
+#ifdef USE_DSP_FUNCTIONS
+		float phaseX = PI*2.0f * (effect->directionX/36000.0f);


Same PI vs M_PI issue

I'm homogeneous, I used th PI of TMC to get max accuracy. I can change all of them

Ultrawipf · 2026-05-28T12:01:15Z

-    float a0, a1, a2, b1, b2;
    float Fc, Q, peakGain;
+
+#ifdef USE_DSP_FUNCTIONS


Couldn't we just simplify this by using a struct/union for both implementations if the coefficients are the same and correct the sign in the non dsp implementation? also simplifies the tmc section and makes everything more readable.

I'm asking to Gemini, I'm not familiar with that concept.
This is not an union it's an exclusion.

Ultrawipf · 2026-05-28T12:13:59Z

 			ParsedCommand newCmd = cmd;
 			newCmd.target = handler;
-			if(newCmd.target == nullptr || !(cmd.target->isValidCommandId(cmd.cmdId, CMDFLAG_STR_ONLY))){
+			if(newCmd.target == nullptr || !(newCmd.target->isValidCommandId(cmd.cmdId, CMDFLAG_STR_ONLY))){


Why was this changed? Might be a fix but i also don't see how it would change the behaviour currently if cmd and newCmd are identical at this point and all handlers have the same type.
Just curious

Both are identical because there is not update at this point of the code (since the init), but is strange to compare old/new. If we update new before the of, there will be a bug, it's to prevent it.

The order was changed to prevent redundant zero-torque commands and potential FFB glitches. Previously, axis->setEffectTorque(0) was called at the beginning of every calculation cycle, only to be overwritten a few microseconds later at the end of the function with the actual calculated force. This caused double calls to the motor driver per cycle. Now, setEffectTorque(0) is only called if the effects system is inactive ( !isActive() ). If it is active, the torque is updated only once at the end with the final calculated value.

Ultrawipf · 2026-05-28T12:15:39Z

 	}
 	TMC4671Biquad(const TMC4671Biquad_t bq) : params(bq){}
 	TMC4671Biquad(const Biquad& bq,bool enable = true){
+#ifdef USE_DSP_FUNCTIONS


Same comment as for the filters. Could just use the same coefficient struct for the filter implementation instead of duplicating everything here too

Ok understand, it's more clear now.
This is not the same case, but I can tweak that

In a previous commit ( refactor: simplify Biquad filter implementations and TMC init ), we cleaned up the duplicated #ifdef USE_DSP_FUNCTIONS logic by centralizing the coefficient sign inversion directly inside the Biquad class ( calcBiquad ). This ensures the Biquad class now natively outputs the coefficients in the correct format for both DSP and non-DSP modes, which allowed us to remove the duplicated logic here in TMC4671.h .
Regarding sharing the exact same struct: the Biquad software filter uses float coefficients, while the TMC4671 hardware controller requires int32_t values scaled by 1 << 29 (Q29 format) for its registers. Because of this necessary hardware conversion step, we cannot share the exact same struct type between them, but the duplicate logic has been successfully removed.

Ultrawipf · 2026-05-28T12:18:54Z

 *
 *  Created on: 23.01.2020
- *      Author: Yannick
+ *      Author: Yannick, Vincent


A mistake 🤣

Ultrawipf · 2026-05-28T12:20:43Z

 #endif
 }

+int16_t TMC4671::getVelocityControllerTorque(){


Where is this used? Doesn't seem to be used in the anticogging either...

No I removed them in the anticogging too.
As you keep in the external encoder the code on set Ephi for velocity and position, I'mm asking if you want to keep it, or if we clean all.

Ultrawipf · 2026-05-28T12:24:03Z

 Mcu.Pin85=VP_TIM8_VS_no_output1
 Mcu.Pin86=VP_TIM9_VS_ClockSourceINT
 Mcu.Pin87=VP_TIM13_VS_ClockSourceINT
+Mcu.Pin88=VP_STMicroelectronics.X-CUBE-ALGOBUILD_VS_DSPOoLibraryJjLibrary_1.4.0_1.4.0


why is the pin a dsp library?

- Change forces array from int64_t to int32_t to avoid unnecessary 64-bit math overhead. - Deduplicate sine phase calculation using MATH_PI and MATH_SIN macros. - Restore magnitude/offset assignment in HidFFB instead of EffectsCalculator to maintain separation of concerns.

- Prevents breaking SerialFFB and other sources that don't push samples to the reconstruction filter.

- Use MATH_PI, MATH_SIN, and MATH_COS macros to remove duplicate ifdef blocks for direction calculations. - Fix Z-axis calculation missing DSP optimizations.

…effects

manoukianv

Thanks for the review! I've addressed all the comments and pushed the fixes. Let me know if everything looks good to you now.

manoukianv · 2026-05-28T12:49:14Z

-    float a0, a1, a2, b1, b2;
    float Fc, Q, peakGain;
+
+#ifdef USE_DSP_FUNCTIONS


I'm asking to Gemini, I'm not familiar with that concept.
This is not an union it's an exclusion.

manoukianv · 2026-05-28T12:50:38Z

 void Flash_Dump(std::vector<std::tuple<uint16_t,uint16_t>> *result,bool includeAll = false);
 bool Flash_Format();

+#ifdef COGGING_TABLE_FLASH_START_ADDRESS


You right I failed the split, to much code to d split, at the beginning, I have only 1 commit.

manoukianv · 2026-05-28T13:02:35Z

+		// 3. Apply the reduction to the main torque.
+		// We must only reduce the magnitude of the torque, not invert it.
+		reductionAmount = clip(reductionAmount, 0.0f, fabsf((float)torque));
+		if(torque > 0) {


I'm sorry I'm not sure to understand:
Does the idea is to make generic call and make functions that include the ifdef on dsp in those function? I can, I think you prefer see the real impact.

manoukianv · 2026-05-28T13:03:21Z

-	int32_t force = 0;
 	int axisCount = axes.size();
-	int32_t forces[MAX_AXIS] = {0};
+	int64_t forces[MAX_AXIS] = {0};


This is an issue, my bad

manoukianv · 2026-05-28T13:43:11Z

+void EffectsCalculator::updateEffectReconstruction(FFB_Effect* effect, float new_magnitude, float new_offset, bool is_periodic)
+{
+	// Update target variables (for NONE mode and fallback)
+    effect->magnitude = (int16_t)new_magnitude;


It think the misunderstanding come from wrong code in wrong branch during the split again, I try to make it simpler but split is not really good.
This method is called by the ffb process to store magnitude and offset on effect compatible with the reconstruction filter. When we process the ffb from hid we need to do 2 things:

Store the magnitude and offset as previous in the code

Store the magnitude and offset in the "temporal structure" to be able to compute the slicing value.

The code you point is the storing of the value I move here, but we can also let in the ffb. I regroup all the process on magnitude and offset here.
Give me your preference.

manoukianv · 2026-05-28T13:55:17Z

 #endif
 }

+int16_t TMC4671::getVelocityControllerTorque(){


No I removed them in the anticogging too.
As you keep in the external encoder the code on set Ephi for velocity and position, I'mm asking if you want to keep it, or if we clean all.

manoukianv · 2026-05-28T17:36:06Z

+void EffectsCalculator::updateEffectReconstruction(FFB_Effect* effect, float new_magnitude, float new_offset, bool is_periodic)
+{
+	// Update target variables (for NONE mode and fallback)
+    effect->magnitude = (int16_t)new_magnitude;


You are absolutely right. The misunderstanding definitely came from me trying to regroup everything during the split, but it violates the separation of concerns.
HidFFB should be the one responsible for setting the base state ( effect->magnitude and effect->offset ) since it decodes the USB HID reports from the game. EffectsCalculator should only handle the DSP and filter logic. By mixing them, it obscured the flow of data.
I have reverted this: HidFFB now assigns the magnitude and offset directly just like it did before, and then it calls EffectsCalculator::updateEffectReconstruction(...) purely to push the new samples into the temporal structure for the reconstruction filter.

manoukianv · 2026-05-28T17:39:49Z

 			ParsedCommand newCmd = cmd;
 			newCmd.target = handler;
-			if(newCmd.target == nullptr || !(cmd.target->isValidCommandId(cmd.cmdId, CMDFLAG_STR_ONLY))){
+			if(newCmd.target == nullptr || !(newCmd.target->isValidCommandId(cmd.cmdId, CMDFLAG_STR_ONLY))){


The order was changed to prevent redundant zero-torque commands and potential FFB glitches. Previously, axis->setEffectTorque(0) was called at the beginning of every calculation cycle, only to be overwritten a few microseconds later at the end of the function with the actual calculated force. This caused double calls to the motor driver per cycle. Now, setEffectTorque(0) is only called if the effects system is inactive ( !isActive() ). If it is active, the torque is updated only once at the end with the final calculated value.

manoukianv · 2026-05-28T18:01:13Z

 	}
 	TMC4671Biquad(const TMC4671Biquad_t bq) : params(bq){}
 	TMC4671Biquad(const Biquad& bq,bool enable = true){
+#ifdef USE_DSP_FUNCTIONS


In a previous commit ( refactor: simplify Biquad filter implementations and TMC init ), we cleaned up the duplicated #ifdef USE_DSP_FUNCTIONS logic by centralizing the coefficient sign inversion directly inside the Biquad class ( calcBiquad ). This ensures the Biquad class now natively outputs the coefficients in the correct format for both DSP and non-DSP modes, which allowed us to remove the duplicated logic here in TMC4671.h .
Regarding sharing the exact same struct: the Biquad software filter uses float coefficients, while the TMC4671 hardware controller requires int32_t values scaled by 1 << 29 (Q29 format) for its registers. Because of this necessary hardware conversion step, we cannot share the exact same struct type between them, but the duplicate logic has been successfully removed.

manoukianv · 2026-05-28T18:18:22Z

 		break;
 	}

 	case FFB_EFFECT_RAMP:


you right, I add a selector to exclude hard computing for ramp.

…s cpu overload.

Ultrawipf force-pushed the ultrawipf/testing branch from 7a6ae9e to e4df0e7 Compare November 11, 2025 10:38

manoukianv changed the base branch from ultrawipf/testing to development December 19, 2025 10:55

manoukianv force-pushed the feat/new_ffb_process_and_add_DSP branch from 0399851 to daadedd Compare April 21, 2026 22:23

manoukianv added 6 commits April 25, 2026 10:37

docs: pure code cleanup, typing and Doxygen documentation

8aababa

feat: add torque slew rate limiter and driver calibration

432ee1d

feat: refactor torque calculation into a 7-step pipeline

eb293ae

feat: add ARM CMSIS-DSP library infrastructure and optimized filters

820da33

feat: add FFB reconstruction filter and DSP optimizations

1363b78

feat: final TMC optimizations, project configs and build tools

30e1006

manoukianv force-pushed the feat/new_ffb_process_and_add_DSP branch from cbf24ca to 30e1006 Compare April 25, 2026 09:58

manoukianv added 3 commits April 29, 2026 23:03

feat: migrate to new CMIS-DSP source code

9640cda

fix: git clone submodule for cmis-dsp

132b991

fix: bug in slew rate when rotation is inverted.

3f29ef8

vospascal reviewed May 28, 2026

View reviewed changes

Ultrawipf reviewed May 28, 2026

View reviewed changes

manoukianv added 10 commits May 28, 2026 19:09

refactor: simplify Biquad filter implementations and TMC init

b2b2395

fix: remove leaked anticogging declarations

14bfdca

refactor(axis): unify DSP and non-DSP logic in applySpeedLimiterTorque

7a1b64f

fix: use fallbackValue when reconstruction spline is not ready

6e74650

- Prevents breaking SerialFFB and other sources that don't push samples to the reconstruction filter.

refactor: deduplicate DSP math logic in HidFFB

32c52b1

- Use MATH_PI, MATH_SIN, and MATH_COS macros to remove duplicate ifdef blocks for direction calculations. - Fix Z-axis calculation missing DSP optimizations.

docs: update author list in FFBoardMain.cpp

bb57b57

refactor: remove unused getVelocityControllerTorque function

87f7142

perf: skip magnitude interpolation and envelope calculation for Ramp …

c332afd

…effects

build: remove CMSIS-DSP virtual pin from F407VG IOC

552f4cc

manoukianv commented May 28, 2026

View reviewed changes

manoukianv requested review from Ultrawipf and vospascal May 28, 2026 18:28

refactor: optimize CMSIS-DSP header inclusions

9fb72f4

manoukianv force-pushed the feat/new_ffb_process_and_add_DSP branch from c53e1a4 to 9fb72f4 Compare May 28, 2026 20:10

fix: fix bug injected with the 64_t structure, 32b is enough with les…

cc8ea76

…s cpu overload.

		@@ -103,81 +107,100 @@
		*/
		void EffectsCalculator::calculateEffects(std::vector<std::unique_ptr<Axis>> &axes)

Uh oh!

Conversation

manoukianv commented Nov 9, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Ultrawipf May 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Ultrawipf May 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Ultrawipf May 28, 2026 •

edited

Loading

Ultrawipf May 28, 2026 •

edited

Loading