Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CMSIS defines are too generic #15047

Closed
ladislas opened this issue Sep 2, 2021 · 7 comments
Closed

CMSIS defines are too generic #15047

ladislas opened this issue Sep 2, 2021 · 7 comments

Comments

@ladislas
Copy link
Contributor

ladislas commented Sep 2, 2021

Description of defect

Some TARGET/CMSIS defines are given names that are really generic such as DMA2D in https://github.com/ARMmbed/mbed-os/blob/master/targets/TARGET_STM/TARGET_STM32F7/STM32Cube_FW/CMSIS/stm32f769xx.h

This prevents the user from defining variables/structs/classes with the same name. In our case we want to have:

namespace interface {

class DMA2D
{
	// ...
};

}

To define the DMA2D interface other components would need.

I could rename it DMA2DBase but this breaks our naming rules.

DMA2D is not the only one and the same issues could arise with JPEG, DSI, DAC, ADC, etc.

Target(s) affected by this defect ?

SM32F769 but others might be as well.

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

arm-none-eabi-c++ (GNU Arm Embedded Toolchain 10-2020-q4-major) 10.2.1 20201103 (release)

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

mbed-os-6.14.0

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

latest

How is this defect reproduced ?

See description on how to reproduce.

@jeromecoutant
Copy link
Collaborator

Hi
Please open an issue in the source repo:
https://github.com/STMicroelectronics/cmsis_device_f7

@0xc0170
Copy link
Contributor

0xc0170 commented Sep 2, 2021

I wonder what naming scheme should there. Everyone just follows their reference manual when creating Peripheral_declaration. I don't think CMSIS dictates anything towards this direction.

One way would be to not allow this MCU header get leaked outside of Mbed OS library (only if anything outside requests it and includes it) or?

@ladislas
Copy link
Contributor Author

ladislas commented Sep 2, 2021

@ladislas
Copy link
Contributor Author

@jeromecoutant sadly the issue in ST's repo was closed for fair reasons.

It seems that you don't just pull the file from there to add to mbed-os, but you do some modifications before.

Would it be possible to add something like this:

/** @addtogroup Peripheral_declaration
  * @{
  */
#ifndef HIDE_PERIPHERAL_DECLARATION

#define TIM2                ((TIM_TypeDef *) TIM2_BASE)
#define TIM3                ((TIM_TypeDef *) TIM3_BASE)
// ...
#define DMA2D               ((DMA2D_TypeDef *)DMA2D_BASE)
// ...
#define JPEG                ((JPEG_TypeDef *) JPEG_BASE)
#define DSI                 ((DSI_TypeDef *)DSI_BASE)

#endif // HIDE_PERIPHERAL_DECLARATION
/**
  * @}
  */

This way it won't break anything in current code and people who need it can just define HIDE_PERIPHERAL_DECLARATION somewhere.

If it works for you I can make a PR :)

@jeromecoutant
Copy link
Collaborator

Hi
Issue is to find an easy way to keep the patch when we update drivers files for each ST Cube FW...
I tried to remove these peripheral declarations, but it seems that lotsv of HAL ST files are based on this... :-(

Ex: https://github.com/STMicroelectronics/STM32CubeF7/blob/master/Drivers/STM32F7xx_HAL_Driver/Inc/stm32f7xx_hal_dac.h#L35
#if defined(DAC)

@ladislas
Copy link
Contributor Author

Ah yes I see... well then I'm not sure there's an easy solution :(

@0xc0170
Copy link
Contributor

0xc0170 commented Oct 4, 2021

I am inclined to close this issue. There's no easy solution and we won't be able to fix all implementations out there.

Closing as won't fix.

@0xc0170 0xc0170 closed this as completed Oct 4, 2021
Issue Workflow automation moved this from Needs Triage to Done Oct 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

No branches or pull requests

4 participants