Skip to content

STM32: Adds missing declarations in periph.h for stm32f767xx #2809

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Apr 25, 2020
Merged

STM32: Adds missing declarations in periph.h for stm32f767xx #2809

merged 1 commit into from
Apr 25, 2020

Conversation

k0d
Copy link

@k0d k0d commented Apr 24, 2020

@hierophect found this bug, it's stopping pulseio from working on the f767.

tannewt
tannewt reviewed Apr 25, 2020
View reviewed changes
ports/stm/peripherals/stm32f7/stm32f767xx/periph.h Outdated
@@ -50,6 +50,8 @@ const mcu_periph_obj_t mcu_uart_rx_list[25];
//Timers
#define TIM_BANK_ARRAY_LEN 14
#define TIM_PIN_ARRAY_LEN 55
TIM_TypeDef * mcu_tim_banks[TIM_BANK_ARRAY_LEN];
extern TIM_TypeDef * mcu_tim_banks[TIM_BANK_ARRAY_LEN];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this array declared in a different header somewhere? That'd be better.

@hierophect
Copy link
Collaborator

hierophect commented Apr 25, 2020 •
edited
Loading

So, FYI, this oversight is present in every SoC that we support, including heavily tested ones like the F405. It doesn't impact the functionality of the timers in any of them as far as I'm aware, which is why I hadn't noticed it before.

The use of the extern keyword here was carried as a convention over from the pins.c and pins.h file, but I'll admit I don't need to make much use of the keyword in my own projects. Is it strictly necessary to even have it at all for array declarations like this?

@k0d
Copy link
Author

k0d commented Apr 25, 2020

I don't really know much about this, just it couldn't find mcu_tim_pin_list when I enabled pulseio, so I assumed it needed that. I'll look into it more to figure out why it needed it.

@hierophect
Copy link
Collaborator

hierophect commented Apr 25, 2020 •
edited
Loading

@k0d what's the nature of the issue you were having with this array not being extern? I want to make sure we aren't overlooking any other issues. If there's a bug here it should be fixed for all boards.
EDIT: ninja'd by your comment - is it possible you missed including periph.h? I've done that a few times.

@k0d
Copy link
Author

k0d commented Apr 25, 2020

ok, that extern isn't needed...it's the other line that was important.

@hierophect
Copy link
Collaborator

Oh, yes I see we're simply missing the tim_pin_list array altogether! That's a typo for sure. Please remove the extern for now, I'll fix it across all boards later.

@k0d k0d force-pushed the stm32f767xx_fix branch from 5dbdb3d to 8814dd0 Compare April 25, 2020 17:51
@hierophect hierophect merged commit 652a5de into adafruit:master Apr 25, 2020
@hierophect
Copy link
Collaborator

Thanks @k0d!

@hierophect hierophect added the stm label Apr 27, 2020
@hierophect hierophect changed the title Adds missing declarations in periph.h for stm32f767xx STM32: Adds missing declarations in periph.h for stm32f767xx Apr 27, 2020
@k0d k0d deleted the stm32f767xx_fix branch April 27, 2020 17:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tannewt tannewt tannewt left review comments

@hierophect hierophect hierophect approved these changes

Assignees
No one assigned
Labels
stm
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@k0d @hierophect @tannewt