search

ARM-software / CMSIS_5

CMSIS Version 5 Development Repository
http://arm-software.github.io/CMSIS_5/index.html
Apache License 2.0
1.35k stars 1.08k forks source link

CMSIS DSP 5.7 PID float32_t controller init faults CPU #1243

Closed GI101 closed 3 years ago

GI101 commented 3 years ago

Hello all,

CCSv9.3 Build for target 7M4 Cortex CPU: 120Mhz CPU, 150 DMIPS, DSPLib-cm4f build, little Endian, FPv4spd16, Code State Thumb-16, TI Compiler v18.12.4.LTS and earlier produce No warnings or errors

Note struct instance name {arm_pid_instance_f32} with (state[3]) for called functions S->[n] data placed into array. Also passing false or even comment memset() will not stop CPU hard faults when called from main.c .

Edit 7/16/21 : Calling arm_sin_cos_f32() within NVIC interrupt vector cause immediate CPU hard fault, explained threads below.

PID init faults CPU when { arm_pid_init_f32() } is called using global pointers via (arm_pid_instance_f32 Ki,Kp,Kd) with .h external globals hits to named pointers for struct items {arm_pid_instance_f32 Kp, Ki, Kd} main.c. No errors on project build to suggest the CPU will fault for calling or creating the init PID instance structure.

The calling c-file pointers are working since PID controller is not faulting NVIC interrupt handler. I also try move init PID into main.c , invoking {arm_pid_init_f32()} faults CPU immediately. What have I missed of how to instantiate and use the PID speed controller? Are there any reports of the float32 type def not working via {rtsv7M4_T_le_v4SPD16_eabi.lib} FPv4SPD16 ?

Thanks for any ideas :-) /* ----------------------------------------------------------------------

Interrupt handler, seems not faulting 7M4 CPU: /**

GI101 commented 3 years ago

Appears other floating point functions cause Cortex 7M4 hard fault NMI upon assertion. The interrupt handler is also a point of failure for one of several DSP (arm_math.h) functions being called. Previously had put (return;) statement at various places in the interrupt handler to determine the function causing a fault. The sin/cos function placed above other DSP 32bit floating point functions was thought to be a culprit. The complex sin/cos tables access may be where little endian FPv4spd16 Thumb16 gets wiggly.

/ Compute the phasor sin/cos / /* Floating-point sin_cos function.

GI101 commented 3 years ago

Suspect faults via sin/cos(): { in = theta * 0.00277777777778f; } Seemingly FP is to many decimal places 7M4 FPU via FPv4spd16? Oddly it fooled me several times IP execution other functions past this point. I seem to recall reading FP16 limits 5 places right of decimal point. /* @brief Floating-point sin_cos function. @param[in] theta input value in degrees @param[out] pSinVal points to processed sine output @param[out] pCosVal points to processed cosine output @return none / void arm_sin_cos_f32( float32_t theta, float32_t pSinVal, float32_t pCosVal) { float32_t fract, in; / Temporary input, output variables / uint16_t indexS, indexC; / Index variable / float32_t f1, f2, d1, d2; / Two nearest output values / float32_t Dn, Df; float32_t temp, findex; / input x is in degrees / / Scale input, divide input by 360, for cosine add 0.25 (pi/2) to read sine table / in = theta * 0.00277777777778f;

GI101 commented 3 years ago

Below is the theta data input to { arm_sin_cos_f32(theta[0], Sin, Cos) } but it seems some DSP 32bit floating functions cause NMI hard fault exception when executed. Oddly CMSIS produced no errors via same compile LTS version, little endian, FPv4spd16, Thumb16. It seems the PID controller init call causes the same NMI fault exception when executed. There are other FP-lib selections for the LTS compiler, which is the recommended one to use with Cortex M4?

Oddly this call {arm_clarke_f32(Ia[0], Ib[0], Ic[0], pIalpha, pIbeta); } precedes the Sin,Cos call and sits above Park transform does not cause exceptions.

The arm_sin_cos_f32(theta[0], Sin, Cos); outputs to Park transform and required a pointer conversion of some kind. Code checker don't seem to care if his output pointer goes into Sin[0], Cos[0] or simply the pointers Sin, Cos as shown below. either way it produce hard fault exception.

Edit 7/16/21: As noted above thread; Suspect suffix (f) added to floating point decimal numbers are not being instantiated as FPU directive as code checker show (f) in black color. 

    /* Compute the phasor sin/cos */
    /* Floating-point sin_cos function.
     * @param[in]     theta    input value in degrees
     * @param[out]    pSinVal  points to processed sine output
     * @param[out]    pCosVal  points to processed cosine output */
    arm_sin_cos_f32(theta[0], Sin, Cos);

    /**
    * Forward Park transform: Ialpha/Ibeta in, pIq/pId out, sin/cos in
    * Floating-point Park transform
    * @param[in]  Ialpha  input two-phase vector coordinate alpha
    * @param[in]  Ibeta   input two-phase vector coordinate beta
    * @param[out] pId     points to output rotor reference frame d
    * @param[out] pIq     points to output rotor reference frame q
    * @param[in]  sinVal  sine value of rotation angle theta
    * @param[in]  cosVal  cosine value of rotation angle theta
    * @return     none         */
    arm_park_f32(Ialpha[0], Ibeta[0], pId, pIq, pSin[0], pCos[0]);

or this way: arm_park_f32(pIalpha[0], pIbeta[0], pId[0], pIq[0], Sin[0], Cos[0]);

GI101 commented 3 years ago

The (f) suffix is easily changed to a (float32_t) in functions but to fix the sin/cos common tables - no freaking way! Note Ca.lib functions turn orange when includes exist but the compiler IDE code checker ignores non-ambiguous characters inside number strings. Seemingly the CPU would somehow have to know it must pass appended (f) values to the math co-processor or FPU? Perhaps need to add #typedef float (float32_t | f) into arm_math.h?

Edit 7/17/21: Adding #typdef float (f) made no difference. Also verify CPAC (FPU) registers have CP11/CP11 Coprocessor Access Privilege enabled for floating point access. Also verified and later enabled the Sin/Cos FP32 tables are built via CMakeLists.txt being set to (ON). If anything like LWIP Ethernet debug, lists ON means the entire list is scanned by linker, OFF skipped. Note: If all tables are included as a default behavior (OFF), why are all the tables then set to OFF status? CMakeLists.txt :

When OFF it is the default behavior : all tables are included.

option(CONFIGTABLE "Configuration of table allowed" ON)

Features which require inclusion of a data table.

Since some tables may be big, the corresponding feature can be

disabled.

Those options are taken into account only when CONFIGTABLE is ON

option(ARM_SIN_COS_F32 "sin cos f32" ON) option(ARM_SIN_COS_Q31 "sin cos q31" OFF) etc....

GI101 commented 3 years ago

Why would the input value be truncated no matter how the data was passed, formed as pointer or array cell? Neither way passes any input data so DEN is the value being processed. Cry uncle is the word comes to mind.

ThetaIntoCoSinFP32 ThetaIntoCoSinFP32-1

christophe0606 commented 3 years ago

@GI101 Thanks for the detailed report.

Table are OFF by default in cmake because the setting is not taken into account except if you enable CONFIGTABLE .

When CONFIGTABLE is ON then you need to select which tables you want to include.

But if a table was not included although you need it you would either have a link error or an error reported by an init function.

Now regarding the floats:

The f suffix is supported since at least C99. It is used for performance reasons : to prevent the compiler from doing cast or operations in double precision.

The literal written in the source code may be more accurate than a float32 but if followed by f, the compiler should correctly generate a float32. In fact, according to the spec, the conversion done at compile time by the compiler from the C source should be equivalent to strtof

So I don't see how this may be the cause of the issues.

CMSIS-DSP is tested with ARM Clang and gcc. Unfortunately, it is not possible to test will all compilers.

Some compilers have this option -fsingle-precision-constant

If you have something similar, can you check if it makes any difference ?

GI101 commented 3 years ago

Hi Christophe,

I get idea of Configtable ON with the single table Cos/Sin ON reduces size of dsplib-cm4f.lib, makes since. The tables can be very large and Intel 2.8Ghz 4 CPU compile remains about as long, perhaps not as much though for 1 table.

The input theta multiply ( in = theta * 0.00277777777778f ) seems to be what causes DEN. Oddly even for input theta value 0.9999 the result is >32 bits wide or 0.002777500000002222, how can that be correct? The idea of CoSin for a 360° circle should remains 360 degrees to infinity as it spins or (+180 alpha / -180 beta) ? The theta degrees as it relates to an RPM conversion should produce <32 bit resolution solution up to, e.g. 40,000.0 RPM / 0.16666666666667 = N degrees theta. The arm_sin_cos_f32( ) should be able to extrapolate a rotating 360° circle into two halves, recycle every 360° back to 0° for new input theta. That may be where the function is not working as it should since the resolution is >32 bits or long long.

We are checking the TI compiler can support (f) suffix but the 7M4 FPU has single precision mode by design.

GI101 commented 3 years ago

The 32bit floats are not large enough to hold the input data and making them double code goes further prior to faulting FPU. The negative (temp) value when exported seems to cause FPU to set FPSCR=(0x10), register is updated after every FP instruction. I wasn't expecting the DSP code to cause FPU exception issue. This goes beyond my math expertise and makes question the CoSin function was ever tested at this point.

Arm_CoSin_inDouble

GI101 commented 3 years ago

The f suffix is supported since at least C99. It is used for performance reasons : to prevent the compiler from doing cast or operations in double precision.

Oddly I find conflicting C++ reason for (f) argument, tells compiler to use double precision via printf() command. f | double;[-1m.dddddd, where the number of d's is given by the precision (default 6).

7M4 FPU: 3.1.5 Floating-Point Unit (FPU) This section describes the Floating-Point Unit (FPU) and the registers it uses. The FPU provides: ■ 32-bit instructions for single-precision (C float) data-processing operations

So a float64_t is a double word but (f) instructs to do a single precision FPU operation? Oddly replacing float32_t with float_64 on variable (in) allows the value to pass into function and multiply. Debug single step jumps over the conversion table directives on both Cos and Sin calculations then hard faults FPU upon the last output step but seems to go further into the function via float64_t before it faults.

christophe0606 commented 3 years ago

I have just tested your value theta = 0.9999f on a M4F with gcc 10q4 and ARM clang AC6.17

I get the following results with both compilers: Sin=0.017451 Cos=0.999848

It is correct (since theta is in degrees).

I have also tested the other value appearing in your report : theta = 0.0123450002f. I am getting:

Sin=0.000215 Cos=1.000000

The results are again correct.

So the function is behaving as it should and the problem must be related to your compiler + environment.

Unfortunately, it means I won't be able to help more.

I am building with (clang):

-mcpu=cortex-m4 --target=arm-arm-none-eabi -Wdouble-promotion -Ofast -ffast-math -DNDEBUG -Wall -Wextra -Werror -mfloat-abi=hard -mlittle-endian -mthumb -mfpu=fpv4-sp-d16

GI101 commented 3 years ago

TI compiler was set to use relaxed floating point, 7M4 FPU is single precision only conversions. I see you added an suffix on the end of the theta input string, will give it a try too. Ideally the float32_t directive for unsigned integers should not require a suffix since the vendors MCU tool chain is specific to the 7M4 silicon revision level, errata and others quirks.

There are no pre-processor providers selected for GCC-q10-4 in the project build settings imported via CMSIS 5.7 CCS folder. This is the vendors tool chain compiler specific to the 7M4 silicon, may give GCC a try when all else fails. Sadly I was not able to get Eclipse IDE for an ST Discovery project to complete Make via ARM GCC-q10-4 compiler. Has a fatal error, compiler could not find input files even specifying make paths via /cygdrive/c/ or C:/ very disappointing tried for week and finally gave up.

Thanks for checking functions, no doubt the FPU is the issue. I will later give an up date as to what caused the issue if it can be resolved by TI engineers or myself. Thanks Again :-)

BTW: Perhaps (Clang) is not using hardware FPU, rather creating floating point operations via C++ directives.... -mv7M4 -me -O1 --opt_for_speed=0 --fp_mode=strict --include_path="C:/ti/ccs910/ccs/tools/compiler/ti-cgt-arm_18.12.4.LTS/include" --include_path="C:/Users/Documents/CCS91/dsplib-cm4f" --include_path="C:/ti/ccs910/ccs/tools/compiler/CMSIS_5-5.7.0/CMSIS/DSP/Include" --include_path="C:/ti/ccs910/ccs/tools/compiler/CMSIS_5-5.7.0/CMSIS/Core/Include" --include_path="C:/ti/ccs910/ccs/tools/compiler/CMSIS_5-5.7.0/CMSIS/DSP/PrivateInclude" --include_path="C:/ti/ccs910/ccs/tools/compiler/CMSIS_5-5.7.0/CMSIS/DSP/Lib/ARM/arm_cortexM4lf_math.lib" --define=FPU_PRESENT=1 --define=ARM_MATH_CM4 --define=__COMPILER_BARRIER --define=TI_ARM__ --define=_INLINE -g --diag_warning=225 --diag_wrap=off --verbose_diagnostics --verbose

GI101 commented 3 years ago

There were few issues causing FPU exception faulting.

  1. CMSIS project build flags runtime support was set for --float_support=FPv4SPD16 but was not in the CDT build summary flags.
  2. Local optimizations level set to 1 was making debug single step into jump over table read directives.
  3. The input data array and output pointers might block either up or down stream pass to data transfers causing exceptions.
  4. The functions text about pointer perhaps could show output arraycell[n] after the fact as to indicate significant direction exists for these pointers. The input cell[n] direction seems obvious enough but output text mimics input design. The data pass to functions and array placements are non forgiving. They don't work with a pointer named in the declarations as static or volatile float32_t *var=0; though may compile without causing any kind of code gen warning or error. Better Example: / Floating-point sin_cos function.
    • @param[in] theta input value in degrees
    • @param pSinVal[out] points to processed sine output as pSinVal
    • @param pCosVal[out] points to processed cosine output as pCosinVal */
christophe0606 commented 3 years ago

Great that you have progressed on finding the root causes for the issues. I assume I can close this ticket now since it is not related to the code ?

GI101 commented 3 years ago

Hi, I still could not get the PID to work but think the typedef instance name being used in my C file as a pointer over to the PID instance. Otherwise just adding #include arm_math.h would not allow PID function to find instance variables Kp,Ki,Kd shown below. PID is in the same interrupt handler but has exception fault when it executes, so I comment around it for now. The PID inti and reset also fault the CPU. Any idea why the C file must add the typedef instance name too? Thanks for your help!

/< The proportional gain. / arm_pid_instance_f32 Kp; /< The integral gain. / arm_pid_instance_f32 Ki; /*< The derivative gain. / arm_pid_instance_f32 *Kd;

/**

Great that you have progressed on finding the root causes for the issues. I assume I can close this ticket now since it is not related to the code ?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/ARM-software/CMSIS_5/issues/1243#issuecomment-883886846, or unsubscribehttps://github.com/notifications/unsubscribe-auth/APPFPCPBQJPSIXP42FI2RZDTYZHENANCNFSM5AHS6LUA.

christophe0606 commented 3 years ago

There are no #if around the definition of arm_pid_instance_f32 in arm_math.h so I don't see why just including arm_math.h is not enough.

arm_pid_instance_f32 is containing Kp, Ki, Kd fields.

I see 3 arm_pid_instance_f32 named Kp, Ki, Kd in your code example ?

Are you sure the pid API is used in the right way ?

I would do:

arm_pid_instance_f32 pidState;
/* Init pidState.Kp, pidState.Ki, pidState.Kd 
Once those fields are initialized, arm_pid_init_f32 can be used
*/
arm_pid_init_f32(&pidState,1);
/* Now the PID can be used */
outSample=arm_pid_f32(&pidState, inSample);
GI101 commented 3 years ago

Point was the PID can not be initialized if the instance is not being recognized by the calling C file, after adding the include arm_math.h . The float32 PID text instructions do not seem to suggest instance can be initiated by the caller by adding an include arm_math.h to the C file of the caller.

Also If I don't add the PID instance name and variable names Kp,Ki,Kd to the calling C file (shared section) the compiler throws exception unresolved symbol const first referenced in double/int --- unresolved symbol/s remain Kp in obj main.c. This Kp the first symbol error, compiler is set to exist on 1st error.

Wonder what I'm missing here??

From: Christophe Favergeon @.> Sent: Friday, July 23, 2021 12:50 AM To: ARM-software/CMSIS_5 @.> Cc: GI101 @.>; Mention @.> Subject: Re: [ARM-software/CMSIS_5] CMSIS DSP 5.7 PID float32_t controller init faults CPU (#1243)

There are no #if around the definition of arm_pid_instance_f32 in arm_math.h so I don't see why just including arm_math.h is not enough.

arm_pid_instance_f32 is containing Kp, Ki, Kd fields.

I see 3 arm_pid_instance_f32 named Kp, Ki, Kd in your code example ?

Are you sure the pid API is used in the right way ?

I would do:

arm_pid_instance_f32 pidState; / Init pidState.Kp, pidState.Ki, pidState.Kd Once those fields are initialized, arm_pid_init_f32 can be used / arm_pid_init_f32(&pidState,1); / Now the PID can be used / outSample=arm_pid_f32(&pidState, inSample);

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/ARM-software/CMSIS_5/issues/1243#issuecomment-885397106, or unsubscribehttps://github.com/notifications/unsubscribe-auth/APPFPCKKEOIKDVRDWFO4IB3TZDYILANCNFSM5AHS6LUA.

GI101 commented 3 years ago

This is how CMSIS instructions suggest to initialize the PID controller. Yet unresolved symbols occur without adding the PID instance name followed by the 3 field names to the caller of the PID function. This error may be due to const variables used in the PID init? Perhaps PID faults CPU due to how the pointer is being named without storage array cell like Co/Sin was doing.

Callers shared section: /< The proportional gain. / arm_pid_instance_f32 Kp; /< The integral gain. / arm_pid_instance_f32 Ki; /*< The derivative gain. / arm_pid_instance_f32 *Kd;

/** The initialization function performs the following operations:
* - Initializes the Gains A0, A1, A2 from Kp,Ki, Kd gains.
* - Zeros out the values in the state buffer. */
//
    arm_pid_init_f32(Kp, true);
    arm_pid_init_f32(Ki, true);
    arm_pid_init_f32(Kd, true);

From: Brett Price @.> Sent: Friday, July 23, 2021 8:19 AM To: ARM-software/CMSIS_5 @.> Subject: Re: [ARM-software/CMSIS_5] CMSIS DSP 5.7 PID float32_t controller init faults CPU (#1243)

Point was the PID can not be initialized if the instance is not being recognized by the calling C file, after adding the include arm_math.h . The float32 PID text instructions do not seem to suggest instance can be initiated by the caller by adding an include arm_math.h to the C file of the caller.

Alos If I don't add the PID instance name and variable names Kp,Ki,Kd to the calling C file (shared section) the compiler throws exception unresolved symbol const first referenced in double/int --- unresolved symbol/s remain Kp in obj main.c. This Kp the first symbol error, compiler is set to exist on 1st error.

Wonder what I'm missing here??

From: Christophe Favergeon @.> Sent: Friday, July 23, 2021 12:50 AM To: ARM-software/CMSIS_5 @.> Cc: GI101 @.>; Mention @.> Subject: Re: [ARM-software/CMSIS_5] CMSIS DSP 5.7 PID float32_t controller init faults CPU (#1243)

There are no #if around the definition of arm_pid_instance_f32 in arm_math.h so I don't see why just including arm_math.h is not enough.

arm_pid_instance_f32 is containing Kp, Ki, Kd fields.

I see 3 arm_pid_instance_f32 named Kp, Ki, Kd in your code example ?

Are you sure the pid API is used in the right way ?

I would do:

arm_pid_instance_f32 pidState; / Init pidState.Kp, pidState.Ki, pidState.Kd Once those fields are initialized, arm_pid_init_f32 can be used / arm_pid_init_f32(&pidState,1); / Now the PID can be used / outSample=arm_pid_f32(&pidState, inSample);

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHubhttps://github.com/ARM-software/CMSIS_5/issues/1243#issuecomment-885397106, or unsubscribehttps://github.com/notifications/unsubscribe-auth/APPFPCKKEOIKDVRDWFO4IB3TZDYILANCNFSM5AHS6LUA.

GI101 commented 3 years ago

Adding below to callers C file fixes the initialization/reset exception faulting. Seem the caller can not simply point to the arm_math.h include to index the instance, caller must provide storage transfer. I had tested below from 3 global exports left in the interrupt handlers C file after moving the PID init/reset functions into main.c , the 3 pointers still threw exceptions. The storage must be located in the callers C file it seems.

I don't yet know if the PID speed controller is any better since the SVM partly sourced from GIT project would not produce the desired PWM duty cycles via Clarke and theta into Arm_Co/Sin() and several other calls to Park/InvClarke() etc.... I sort of got frustrated trying to control small float values from -1 to +1 when PWM did not produce current flow in the inverter. Assume the float32 Co/Sin() has same magnitude limits as the fixed point Co/Sin but that was not made explicitly clear in the text. For now have to give this idea a rest, it's been few weeks trying to get it to work.

Callers shared section: /< The proportional gain. */ arm_pid_instance_f32 Kp[0]; /*< The integral gain. / arm_pid_instance_f32 Ki[0]; /< The derivative gain. */ arm_pid_instance_f32 Kd[0];