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

Added: display xCoreID in vTaskList #2064

Closed
wants to merge 2 commits into from
Closed

Conversation

dmcnaugh
Copy link
Contributor

Proposed solution for [TW#16439] FreeRTOS xTaskList should be extended to show xCoreID for SMP #1260

Copy link
Member

@igrr igrr left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @dmcnaugh, the feature looks good! I left a comment related to maintaining compatibility.

@@ -181,6 +181,7 @@ typedef struct xTASK_STATUS
uint32_t ulRunTimeCounter; /*!< The total run time allocated to the task so far, as defined by the run time stats clock. See http://www.freertos.org/rtos-run-time-stats.html. Only valid when configGENERATE_RUN_TIME_STATS is defined as 1 in FreeRTOSConfig.h. */
StackType_t *pxStackBase; /*!< Points to the lowest address of the task's stack area. */
uint32_t usStackHighWaterMark; /*!< The minimum amount of stack space that has remained for the task since the task was created. The closer this value is to zero the closer the task has come to overflowing its stack. */
BaseType_t xCoreID;
Copy link
Member

Choose a reason for hiding this comment

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

This line needs a comment similar to the lines above

Copy link

Choose a reason for hiding this comment

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

After looking BaseType_t definition in portmacro.h, BaseType_t type is int. I think uint32_t is more suitable for xCoreID.

#define portBASE_TYPE	int
typedef portBASE_TYPE			BaseType_t;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, I have taken made xCoreID have type BaseType_t because this is how the value is typed in the tskTaskControlBlock (tasks.c) where the value is sourced from.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Comment now added

@@ -4449,7 +4450,7 @@ For ESP32 FreeRTOS, vTaskExitCritical implements both portEXIT_CRITICAL and port
pcWriteBuffer = prvWriteNameToBuffer( pcWriteBuffer, pxTaskStatusArray[ x ].pcTaskName );

/* Write the rest of the string. */
sprintf( pcWriteBuffer, "\t%c\t%u\t%u\t%u\r\n", cStatus, ( unsigned int ) pxTaskStatusArray[ x ].uxCurrentPriority, ( unsigned int ) pxTaskStatusArray[ x ].usStackHighWaterMark, ( unsigned int ) pxTaskStatusArray[ x ].xTaskNumber );
sprintf( pcWriteBuffer, "\t%c\t%u\t%u\t%u\t%hd\r\n", cStatus, ( unsigned int ) pxTaskStatusArray[ x ].uxCurrentPriority, ( unsigned int ) pxTaskStatusArray[ x ].usStackHighWaterMark, ( unsigned int ) pxTaskStatusArray[ x ].xTaskNumber, ( int ) pxTaskStatusArray[ x ].xCoreID );
Copy link
Member

Choose a reason for hiding this comment

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

Some existing code may rely on the format of the string returned by vTaskList. Suggest introducing a new sdkconfig option, e.g. CONFIG_FREERTOS_VTASKLIST_INCLUDE_COREID, disabled by default. Then put the modifications under ifdef, so that by default the old behavior is preserved.

Copy link

Choose a reason for hiding this comment

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

And here int could be "unsigned int" or "uint32_t" following above comment.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added the option CONFIG_FREERTOS_VTASKLIST_INCLUDE_COREID as requested. Please se the revised Pull Request.

@dmcnaugh
Copy link
Contributor Author

dmcnaugh commented Jul 8, 2018

Thanks for approving this.
What happens from here: when and/or how does this PR show up in the master branch"?

@igrr
Copy link
Member

igrr commented Jul 9, 2018

Hi @dmcnaugh, sorry for the slow turnaround. I've put this into review/merge queue, the changes should be in master branch in a few days.

@igrr igrr added the Status: Pending blocked by some other factor label Jul 9, 2018
igrr pushed a commit that referenced this pull request Jul 28, 2018
@igrr
Copy link
Member

igrr commented Jul 28, 2018

Cherry-picked in 0fb1945, thanks!

@igrr igrr closed this Jul 28, 2018
@igrr igrr removed the Status: Pending blocked by some other factor label Aug 9, 2018
catalinio pushed a commit to catalinio/pycom-esp-idf that referenced this pull request Jun 28, 2019
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.

2 participants