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

Allow not to use vTaskDelete() #51

Merged
merged 1 commit into from
Mar 1, 2023

Conversation

JalonWong
Copy link
Contributor

@JalonWong JalonWong commented Feb 22, 2023

Currently, if INCLUDE_vTaskDelete is set to 0, the compilation will fail.

rust-lld: error: undefined symbol: freertos_rs_delete_task
>>> referenced by task.rs:167 (src\task.rs:167)

@schteve
Copy link
Collaborator

schteve commented Feb 24, 2023

I don't like the approach of modifying the internals of the function - if the user was trying to not include it, but the function is still available from the Rust side, it could result in some weird behavior for them. I'd much rather gate it with a feature flag on the Rust side since that's the approach we already took elsewhere.

Right now it looks like freertos_rs_delete_task() is only used when spawning a task so maybe that's easy to do. We have had issues in the past where the linker still complains, and I'm not sure why - would be nice to learn more.

@JalonWong
Copy link
Contributor Author

Add a feature in Cargo.toml, is this appropriate?

@schteve
Copy link
Collaborator

schteve commented Feb 24, 2023

Yes you've got the idea, I would prefer the logic to not be negative here (cfg(not(not_delete_task)) -> cfg(delete_task)). This also means the feature is needed in the default list too.

@JalonWong
Copy link
Contributor Author

Yes you've got the idea, I would prefer the logic to not be negative here (cfg(not(not_delete_task)) -> cfg(delete_task)). This also means the feature is needed in the default list too.

Modified

@schteve schteve merged commit 698a1f1 into lobaro:master Mar 1, 2023
@schteve
Copy link
Collaborator

schteve commented Mar 1, 2023

Thanks!

@JalonWong JalonWong deleted the feature/delete_task branch March 3, 2023 02:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants