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

AP_HAL_ESP32: ensure task is deleted if member proc returns #29010

Merged
merged 1 commit into from
Jan 5, 2025
Merged

AP_HAL_ESP32: ensure task is deleted if member proc returns #29010

merged 1 commit into from
Jan 5, 2025
+3 −0

Conversation

srmainwaring
Copy link
Contributor

Resolve issue where an illegal instruction error occurs if a task member function returns.

  • FreeRTOS task functions must not return or exit while the task is active.
  • vTaskDelete(NULL) deletes the calling task.

See:

Motivation

To replicate the issue:

  • Requires AP_DDS: configuration fixes #29009.
  • Configure esp32s3empty with the --enable-dds flag, build and flash.
  • If the DDS transport initialisation fails, or the AP_DDS_Client::main_loop otherwise returns, an illegal instruction occurs.

@srmainwaring
AP_HAL_ESP32: ensure task is deleted if member proc returns
dbf9e5f 
- FreeRTOS task functions must not return or exit.
- vTaskDelete(NULL) deletes the calling task.

Signed-off-by: Rhys Mainwaring <[email protected]>
tpwrules
tpwrules approved these changes Jan 5, 2025
View reviewed changes
Copy link
Contributor

@tpwrules tpwrules left a comment

Choose a reason for hiding this comment

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

Correct according to the docs, tested that it crashes before and doesn't crash now. Thanks for the replication instructions.

It looks like all our other uses of these task functions have an infinite loop and don't return so they don't need adjustment.

@tpwrules tpwrules force-pushed the prs/pr-eps32-delete-task branch 2 times, most recently from 7d3745b to dbf9e5f Compare January 5, 2025 19:56
@tpwrules
Copy link
Contributor

tpwrules commented Jan 5, 2025

Sorry, minor github stupidity...

@tpwrules tpwrules merged commit 8d4d575 into ArduPilot:master Jan 5, 2025
16 checks passed
davidbuzz
davidbuzz reviewed Jan 5, 2025
View reviewed changes
Copy link
Collaborator

@davidbuzz davidbuzz left a comment

Choose a reason for hiding this comment

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

reviewed

@srmainwaring srmainwaring deleted the prs/pr-eps32-delete-task branch January 6, 2025 10:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@davidbuzz davidbuzz davidbuzz left review comments

@tpwrules tpwrules tpwrules approved these changes

Assignees
No one assigned
Labels
BUG esp32 MergeOnCIPass
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@srmainwaring @tpwrules @davidbuzz