Skip to content

Add supervisor.get_previous_traceback() function. #5037

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

Merged
merged 2 commits into from
Jul 23, 2021
Merged

Add supervisor.get_previous_traceback() function. #5037

merged 2 commits into from
Jul 23, 2021

Conversation

hierophect
Copy link
Collaborator

New PR to avoid force pushing rebase over the old one. In retrospect, I don't think this needs any special interaction with deep sleep, since it's really for passing tracebacks and deep sleep exceptions don't have those. If code needs to detect if the last exception was deepsleep vs startup, it should just use the alarm module as per usual. Tested on a Feather M4 Express, works as expected. Useful for #1084.

Heads up for @tannewt to take a look at the string stuff in this since I'm not sure I'm qualified to comment on whether it's safe. Also to double check that this doesn't have any negative interactions with the new structure of status LED - we don't have any exception specific colors anymore, right?

@tannewt
Copy link
Member

tannewt commented Jul 22, 2021

Heads up for @tannewt to take a look at the string stuff in this since I'm not sure I'm qualified to comment on whether it's safe.

I'll do my best. :-)

Also to double check that this doesn't have any negative interactions with the new structure of status LED - we don't have any exception specific colors anymore, right?

Correct. Just double red flash.

Copy link
Member

@tannewt tannewt left a comment

Choose a reason for hiding this comment

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

Looks good to me! Thank you @cwalther and @hierophect

@tannewt tannewt merged commit beda1f7 into adafruit:main Jul 23, 2021
@cwalther
Copy link

Thank you @hierophect!

@tannewt, if the string stuff looks good to you, I guess the TODO comments can be removed? I can make a PR to that effect.

diff --git a/shared-bindings/supervisor/__init__.c b/shared-bindings/supervisor/__init__.c
index 83b473616..055b3a908 100644
--- a/shared-bindings/supervisor/__init__.c
+++ b/shared-bindings/supervisor/__init__.c
@@ -267,14 +267,13 @@ MP_DEFINE_CONST_FUN_OBJ_0(supervisor_ticks_ms_obj, supervisor_ticks_ms);
 //|     ...
 //|
 STATIC mp_obj_t supervisor_get_previous_traceback(void) {
-    // TODO is this a safe and proper way of making a heap-allocated string object that points at long-lived (and likely long and unique) data outside the heap?
     if (prev_traceback_allocation) {
         size_t len = strlen((const char *)prev_traceback_allocation->ptr);
         if (len > 0) {
             mp_obj_str_t *o = m_new_obj(mp_obj_str_t);
             o->base.type = &mp_type_str;
             o->len = len;
-            // TODO is it a good assumption that callers probably aren't going to compare this string, so skip computing the hash?
+            // callers probably aren't going to compare this string, so skip computing the hash
             o->hash = 0;
             o->data = (const byte *)prev_traceback_allocation->ptr;
             return MP_OBJ_FROM_PTR(o);

Also, no need to make it optional anymore? All the better!

@tannewt
Copy link
Member

tannewt commented Jul 23, 2021

Yes, please make a PR @cwalther. Thanks!

@hierophect hierophect deleted the getprevtraceback branch July 26, 2021 17:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants