pbdrv/uart_debug_first_port: (EV3) Log on panic. #437
Conversation
jaguilar
force-pushed
the
dump_log_on_panic
branch
from
bdf4979 to
6951b91
Compare
It's helpful when you crash to see what happens right before you
crash. For example, if you add a log message "about to dereference
sketchy pointer" and then you get a segfault, you'd love to know
whether that message occurred. This change makes it so that at least
on EV3, we will.
This also adds "pb_log" as an alias for pbdrv_uart_debug_printf.
Usually, platforms make it easy to log debug messages, and we should
be no different. C.f. `ESP_LOGI`, Zephyr's `LOG_ERR`, glog's
`LOG(WARNING)` etc.
Tested by adding the following code to pbdrv_init.
```
pb_log("test\n");
lwrb_t* rb = (lwrb_t*)0x1281441490;
pb_log("%p\n", lwrb_get_free(rb));
```
Before this, the "test" would not appear before the panic.
After, it does. If a panic occurs while some messages are pending
in the uart send state, some bytes may be duplicated on the log.
jaguilar
force-pushed
the
dump_log_on_panic
branch
from
6951b91 to
9f46367
Compare
|
Thanks! I think this fine. Maybe the alias could still keep the word UART in it? I'll be adding USB debugging in an upcoming commit. This will be helpful for platforms without uart (like NXT) and for most people without custom cables. Output will go into the existing stdout ring buffer, so it simply shows up in Pybricks Code without custom terminal tools. Since the EV3 reconnect itself on reboot, this should even be convenient for our own everyday use cases. They can be complementary - we'll still want the UART version for sketchy pointers and other reasons for panic 😄. |
I'd prefer not. The person doing the logging cares only about the intent that a message be logged. The transport is not part of the intent. Ideally, pbdrv_log would go to the UART when it makes sense, or the USB interface when it makes sense.
Awesome.
It would be interesting if the stream could be adjusted to denote whether the message is a log message our a python print message. If it were, we could display log messages and user-level messages separately. I guess this isn't really that important since logging won't be enabled on production builds. |
It matters somewhat because you either have that cable or not. It's not all that different from log levels which are quite common. In any case, I don't really mind 🙂. I think we'll mostly still be doing it on a per module basis rather than turning all logging on globally.
We could do that by giving it its own event type, but I would rather keep it very simple so it shows up in Pybricks Code without special adjustments. We could always prefix messages with [debug] if you really needed. We occasionally ask users to try things that we can't reproduce, and something like this has been lacking so far. Once enabled, it should be easy for us to make a build that logs the entire sensor output for a few seconds, for example. |
I guess my point is that pbdrv_uart_log would imply not logging to the USB. But in fact, when I write pbdrv_log, I want it to log to whatever logging device I have available and connected. For example, you could also imagine a function that queues the log to be sent to a bluetooth serial port on a nearby PC. I could have a teleplot window open, listening to that serial port. I don't want to have to go through the code and change every pbdrv_uart_log to pbdrv_bt_log to make that work. I just want to connect the bluetooth logger and get the same output I'd otherwise get from the UART. See what I mean? That being said, I understand I'm in y'all's house. If you want the name changed, comment again to this effect and I will change it. |
4 participants
It's helpful when you crash to see what happens right before you crash. For example, if you add a log message "about to dereference sketchy pointer" and then you get a segfault, you'd love to know whether that message occurred. This change makes it so that at least on EV3, we will.
This also adds "pb_log" as an alias for pbdrv_uart_debug_printf. Usually, platforms make it easy to log debug messages, and we should be no different. C.f.
ESP_LOGI, Zephyr'sLOG_ERR, glog'sLOG(WARNING)etc.Tested by adding the following code to pbdrv_init.
Before this, the "test" would not appear before the panic. After, it does. If a panic occurs while some messages are pending in the uart send state, some bytes may be duplicated on the log.
This only makes this change for EV3. I will separately look into whether it might be useful on other platforms.