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

Add Trace Hook Macros to all API calls #786

Merged
merged 16 commits into from
Sep 20, 2023

Conversation

Techcore123
Copy link
Contributor

Add Trace Hook Macros to all API calls

Description

This pull-request adds out-of-the-box support for different tracing tools.
For more information see following forum post: https://forums.freertos.org/t/add-tracing-functionality-for-all-api-calls/18007.

New trace hook macros have been added for all public API functions, one for each function entry and one for each exit.
There are no functional changes, as the macros are by default empty.

Test Steps

N/A

Checklist:

  • I have tested my changes. No regression in existing tests.
  • I have modified and/or added unit-tests to cover the code changes in this Pull Request.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@Techcore123 Techcore123 requested a review from a team as a code owner September 8, 2023 13:07
@rawalexe
Copy link
Member

rawalexe commented Sep 8, 2023

Thank you for your contribution. I have informed team about your PR and the subject matter expert will be reviewing it soon.

Thank you for your patience.

@archigup
Copy link
Member

archigup commented Sep 8, 2023

Is this intended just for user code to kernel code transitions? In that case it could also potentially be implemented by wrapping FreeRTOS API functions with a macro that adds tracing code; this would also catch code in return statements.

For example:

#include "FreeRTOS.h"
#include "task.h"

BaseType_t trace_xTaskDelayUntil( TickType_t *pxPreviousWakeTime,
                                  const TickType_t xTimeIncrement )
{
    BaseType_t returnVal;
    // entry trace code
    returnVal = xTaskDelayUntil( pxPreviousWakeTime, xTimeIncrement );
    // exit trace code
    return returnVal;
}

#define xTaskDelayUntil trace_xTaskDelayUntil

That could potentially be put in a wrapper header or something similar.

Not advocating for any particular solution, but would like to understand the options that are on the table, and what factors we need to consider.

@rawalexe
Copy link
Member

rawalexe commented Sep 8, 2023

Hello @Techcore123,
Can you please provide a test document, showing that there are no spelling errors on parameters for the trace defined functions and match their ifndef and code implementation

@codecov
Copy link

codecov bot commented Sep 10, 2023

Codecov Report

Patch coverage: 66.66% and project coverage change: -0.74% ⚠️

Comparison is base (c59ce22) 94.35% compared to head (e189e4d) 93.62%.
Report is 1 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #786      +/-   ##
==========================================
- Coverage   94.35%   93.62%   -0.74%     
==========================================
  Files           6        6              
  Lines        2443     2508      +65     
  Branches      598      598              
==========================================
+ Hits         2305     2348      +43     
- Misses         85      107      +22     
  Partials       53       53              
Flag Coverage Δ
unittests 93.62% <66.66%> (-0.74%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
event_groups.c 82.23% <ø> (ø)
list.c 100.00% <ø> (ø)
stream_buffer.c 96.96% <ø> (ø)
tasks.c 94.81% <ø> (ø)
queue.c 91.91% <65.62%> (-2.20%) ⬇️
timers.c 98.77% <100.00%> (+<0.01%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@Techcore123
Copy link
Contributor Author

Hi @rawalexe,
Such a document does not exist yet, but could of course be created.
Are there any guidelines or an example?

@rawalexe
Copy link
Member

rawalexe commented Sep 12, 2023

Hello @Techcore123 ,
The easiest document will be a demo that implements these trace macros, if that's a feasible option for you.

Best Regards,
Anubhav Rawal

@aggarg
Copy link
Member

aggarg commented Sep 19, 2023

I did the following changes:

  1. Wrote a script to find public APIs which did not have trace macros and added those.
  2. Wrote a script to find APIs which were passing wrong parameters and fixed those.
  3. Tested by enabling all the config options.

Here are the scripts that I wrote - Scripts.zip

Signed-off-by: Gaurav Aggarwal <[email protected]>
@aggarg aggarg merged commit 83861f5 into FreeRTOS:main Sep 20, 2023
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.

6 participants