-
Notifications
You must be signed in to change notification settings - Fork 43
Add reserved memory reporting #228
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
Conversation
2a6a366
to
fd242d5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Besides the stray line, everything looks good!
mbed_greentea/mbed_report_api.py
Outdated
@@ -17,6 +17,8 @@ | |||
Author: Przemyslaw Wirkus <[email protected]> | |||
""" | |||
|
|||
from mbed_test_api import TEST_RESULT_OK |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you remove this line? This doesn't look needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bump, wanna remove this?
re_tc_thread_info = re.compile(r"^\[(\d+\.\d+)\][^\{]+\{\{(__thread_info);\"([A-Fa-f0-9\-xX]+)\",(\d+),(\d+)\}\}") | ||
for line in output.splitlines(): | ||
m = re_tc_max_heap_usage.search(line) | ||
if m: | ||
_, _, max_heap_usage = m.groups() | ||
max_heap_usage = int(max_heap_usage) | ||
|
||
m = re_tc_reserved_heap.search(line) | ||
if m: | ||
_, _, reserved_heap = m.groups() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Alternatively you could do m.group(2)
. But it is just a cosmetic comment. Don't worry about it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah good to know, I was just matching the existing code snippet (above)
To be honest even I don't know this side of Greentea. If you have tested it well than I am ok. |
@geky This needs a rebase |
no it doesn't :P |
Oh oops :) |
mbed_greentea/mbed_test_api.py
Outdated
max_thread['max_stack_usage_total'] = max_stack_usage_total | ||
max_thread['reserved_stack_total'] = reserved_stack_total | ||
max_thread['reserved_stack_total'] = reserved_stack_total |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like this got duplicated, mind removing it?
mbed_greentea/mbed_test_api.py
Outdated
@@ -573,6 +585,7 @@ def get_thread_stack_info_summary(thread_stack_info): | |||
'max_stack_size': max_thread_info['stack_size'], | |||
'max_stack_usage': max_thread_info['max_stack'], | |||
'max_stack_usage_total': max_thread_info['max_stack_usage_total'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line is missing a comma at the end.
453f060
to
1a3ad86
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks lovely!
Note: I have no idea what I'm doing!
But we are looking to add the reporting of the reserved memory regions (heap and stack). This seemed to do it, but feel free to be critical of our approach.
Sidenote: we're also looking to add the full memory regions (total rom/ram on a device) but this info is currently not available.
dependent on #223, ARMmbed/mbed-os#4504
commit range: 2a6a366
cc @studavekar, @bridadan