ARM-software / CMSIS_5

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

Versioning files in the files (DSP in this case) - breaking change in minor update? #763

Closed 0xc0170 closed 4 years ago

0xc0170 commented 4 years ago

We identified at least one breaking change in the header https://github.com/ARM-software/CMSIS_5/blob/develop/CMSIS/DSP/Include/arm_math.h. Checking the version logs https://www.keil.com/pack/doc/CMSIS/DSP/html/ChangeLog_pg.html, 1.6.0 should have been 2.0.0.

There's revision/version in Mbed OS 1.5.1. If we update to v1.6.0, the code does not compile (NXP driver fails with build errors). This brings a question how this version should be read. To my understanding, version 1.6.0 should compile if 1.5x did.

Errors we get after update from ARMCC6:

[ERROR] ./mbed-os/targets/TARGET_NXP/TARGET_MCUXpresso_MCUS/TARGET_LPC55S69/drivers/fsl_powerquad_cmsis.c:398:13: warning: assigning to 'q31_t *' (aka 'int *') from 'const q31_t *const' (aka 'const int *const') discards qualifiers [-Wincompatible-pointer-types-discards-qualifiers] cosFact = S->pCosFactor;

IAR8:

[Error] fsl_powerquad_cmsis.c@398,14: [Pe513]: a value of type "q31_t const *" cannot be assigned to an entity of type "q31_t *"

JonatanAntoni commented 4 years ago

Can you please explain more details about the issue you are facing? The source files having the issue are NXP ones and not part of CMSIS-DSP.

Which change broke the backward compatibility from your point of view?

0xc0170 commented 4 years ago

Can you please explain more details about the issue you are facing? The source files having the issue are NXP ones and not part of CMSIS-DSP.

NXP implemented functionality for v1.5, the same version we have currently in Mbed OS. It builds without errors. This is the pull request: https://github.com/ARMmbed/mbed-os/pull/12040 to update the header file to v1.6

If we update the header (arm_math.h) to a version v1.6 , it breaks as I shared. Fixing Const Correctness in v1.6 is a breaking change, isn't it?

There are couple of options to go with:

I think the second option is preferable in this case.

0xc0170 commented 4 years ago

DSP is a component (a library, with an implementation), that answers the question about versioning.