search

ARMmbed / mbed-os

Arm Mbed OS is a platform operating system designed for the internet of things
https://mbed.com
Other
4.67k stars 2.98k forks source link

arm math: DSP header file should be part of CMSIS DSP, not alone in cmsis #12054

Closed 0xc0170 closed 4 years ago

0xc0170 commented 4 years ago

Description of defect

File arm_math.h: https://github.com/ARMmbed/mbed-os/blob/master/cmsis/TARGET_CORTEX_M/arm_math.h is part of this repository. It was added 106f34e2edd89dc385bd501d8d180d2f53d51e87 but I believe it was in the tree longer as we used to have DSP library part of Mbed 2. I recall this header contained some implementation we used (CLZ for example) but that was removed in this header. We should not need this header file.

It causes a problems, as CMSIS DSP is a component (providing implementation), not any API we could overwrite. We should review this header file, and possibly remove it (it is outdated, our import script ignores it). Plus it causes a conflicts, for applications that use DSP, our outdated header file gets in and causes errors.

Ideally, DSP gets in via separate module as we do not provide this functionality in Mbed OS.

Target(s) affected by this defect ?

Cortex-M targets (all)

Toolchain(s) (name and version) displaying this defect ?

All, generic file

What version of Mbed-os are you using (tag or sha) ?

5.14.0

What version(s) of tools are you using. List all that apply (E.g. mbed-cli)

Not relevant

How is this defect reproduced ?

Not relevant

cc @ARMmbed/mbed-os-core

kjbracey commented 4 years ago

We have had a small non-zero number of queries about why we don't include the CMSIS DSP/NN libraries.

Would be nice to bring them in as a feature/component/whatever to make people's life easy.

But certainly we shouldn't be supplying this old fragment that interferes with someone bringing them in themselves.

ciarmcom commented 4 years ago

Internal Jira reference: https://jira.arm.com/browse/MBOTRIAGE-2454

0Grit commented 4 years ago

@AGlass0fMilk

AGlass0fMilk commented 4 years ago

My solution up to now was to find which old version of CMSIS DSP that header was from and import the appropriate source 😅.

It's probably easier to just have people import CMSIS DSP/NN themselves (using mbed add or something). That way Mbed-OS doesn't have to maintain a copy/link to another repo. If they're using DSP/NN (fairly advanced) they will likely be familiar with adding a library.

What I would like to see is a C++ DSP framework for Mbed built with CMSIS DSP :)

0Grit commented 4 years ago

yeah would be nice if CMSIS-packs were mature enough to be used for dependency management. Or if an official and well specified C/C++ dependency management tool were selected to be used with Mbed...

0xc0170 commented 4 years ago

It's probably easier to just have people import CMSIS DSP/NN themselves (using mbed add or something). That way Mbed-OS doesn't have to maintain a copy/link to another repo. If they're using DSP/NN (fairly advanced) they will likely be familiar with adding a library.

That is the plan, this header file was there from times when we had DSP in the tree. It causes issues now.

yeah would be nice if CMSIS-packs were mature enough to be used for dependency management. Or if an official and well specified C/C++ dependency management tool were selected to be used with Mbed...

Wish the same 👍