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.

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

[BUG] Inconsistent parameter name #1165

Closed
swaldhoer opened this issue Oct 25, 2024 · 12 comments
Closed

[BUG] Inconsistent parameter name #1165

swaldhoer opened this issue Oct 25, 2024 · 12 comments
Labels
bug Something isn't working

Comments

@swaldhoer
Copy link
Contributor

Describe the bug
In FreeRTOS version V10.5.0 a parameter in function MPU_vTimerSetReloadMode was changed from UBaseType_t uxAutoReload to BaseType_t xAutoReload :

FreeRTOS-Kernel/History.txt

Lines 414 to 417 in 7215c89

+ Changed uxAutoReload parameter in timer functions to xAutoReload. The
type is now BaseType_t. This matches the type of pdTRUE and pdFALSE.
The new function xTimerGetAutoReload() provides the auto-reload state as
a BaseType_t. The legacy function uxTimerGetAutoReload is retained with the

But there are still old namings used which not all compilers and coding standards are happy about.

Target

  • Development board: n. a.
  • Instruction Set Architecture: n. a.
  • IDE and version: n. a.
  • Toolchain and version: n. a.

Host

  • Host OS: n. a.
  • Version: n. a.

To Reproduce
See https://github.com/search?q=repo%3AFreeRTOS%2FFreeRTOS-Kernel%20uxAutoReload%20&type=code

Expected behavior
Parameter name is in all declarations and definitions xAutoReload.
@swaldhoer swaldhoer added the bug Something isn't working label Oct 25, 2024
@archigup
Copy link
Member

Hey, this does seem like a bug.

Feel free to open a PR if you would like, else we can do so.

@kstribrnAmzn kstribrnAmzn mentioned this issue Oct 30, 2024
Merged
2 tasks
@kstribrnAmzn
Copy link
Member

Went ahead and made the fix - #1166

@kar-rahul-aws
Copy link
Member

Hi @swaldhoer
The PR #1166 has been approved and merged. Does it fix the issue you reported?

@kstribrnAmzn
Copy link
Member

Closing since the issue is fixed.

@swaldhoer
Copy link
Contributor Author

No, it's only partially fixed, see https://github.com/search?q=repo%3AFreeRTOS%2FFreeRTOS-Kernel%20uxAutoReload&type=code

@kstribrnAmzn
Copy link
Member

Thanks for the search! I see I forgot at least a couple which need to be fixed.

@kstribrnAmzn kstribrnAmzn reopened this Nov 1, 2024
@swaldhoer
Copy link
Contributor Author

Yes, but for the remaining uxAutoReload it's not just about the name, but also the type.

@kstribrnAmzn
Copy link
Member

Agreed! I'm going to revert my commit, figure out how we got the mismatched types across the mpu wrapper v1 and v2, and then will come up with a fix commit.

This was referenced Nov 1, 2024
Closed
Merged
@kstribrnAmzn
Copy link
Member

The type mismatches were straightforward so I've created a PR to apply the type mismatch fix - #1181

@kstribrnAmzn
Copy link
Member

All problems should be fixed now - let me know if you still find any other issues.

@swaldhoer
Copy link
Contributor Author

Thanks, will have a look, when updating to the latest version, but I think it should be good!

Small question: The changelog has not been updated on purpose? Is it only updated on new releases or how does it work?

@kstribrnAmzn
Copy link
Member

I took a look at the history.txt log but it only seems to document changes between releases. We will mention this fix between as part of our next release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@swaldhoer @archigup @kstribrnAmzn @kar-rahul-aws