Add uptime to statistics menu #4397
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Don't check the accelerometer on XL before picking a tool. BFW-5689
The "Successfully connected to:" title text was truncated. BFW-5580
PhasesCrashRecovery::_last was not invalid :/ BFW-5690
This does not fully fix the issue, but should help it BFW-5645
The previous one was probably too permissive, possibly causing unwanted resumes. BFW-5645
Normally, this is not a problem. However, in the case that one of the unused extruder thermistor is broken and is reporting values below zero, the printer gets stuck in the reheating state. And yes, this actually happened. BFW-5657
Here's 50 B of flash for ya BFW-5686
-152 B flash BFW-5686
Duh. The problem is that the homeaxis_single_run reports axis as homed even though the homeaxis as a whole fails. This led to XY selftest not catching up the homing fail, resulting in a redscreen. BFW-5686
There were some left overs from where motor detection was part of Mk4 selftest - now handled by ScreenPrinterSetup. Also, add homing detection for all printers. We want to know if homing fails everywhere. BFW-5686
Testing did't like the red cross when you retry the test :P BFW-5686
BFW-5711
We need this to debug the gcode corruption. BFW-5714
BFW-5714
HWLIMIT_NORMAL_MAX_ACCELERATION was previouvly increased, but HWLIMIT_NORMAL_ACCELERATION was forgotten on the low values. BFW-5494 BFW-5644
BFW-5494 BFW-5644
It is enough to just limit max_acceleration_mm_per_s2 and max_feedrate_mm_s, because acceleration, retract_acceleration and travel_acceleration are limited by these anyway. BFW-5644
Under some circumstances (plain-gcode file of the correct size), we could reach a situation where we do have the whole file downloaded, but the download itself is capable of providing some more data, therefore claims „Continue“. We would then first finalize the file and then continue the attempts to download it further, resulting in access to a null pointer. We could probably take more care of setting the range header more thoroughly in such situation, but the server can send more data anyway, so better protect us this way. BFW-5859.
As a result, don't ask for the 4th nozzle's diameter on MMU printer when printing from the 4th filament slot. BFW-5873.
If a file fails the valid for print check not because it is still not downloaded, but because it is somehow broken, report it as such. This goes all the way to the GUI, instead of the „Downloading“ screen. BFW-5830.
Spaces before marks are grammatically incorrect in German
Support singly and doubly quoted INI strings, e.g. leading and trailing spaces in WiFi SSID/PSK field values.
This is necessary for Windows compatibility
Fixes prusa3d#4392 Add uptime to the statistics menu. * Add a new function `RecordUptime` in `src/common/app_metrics.cpp` to record the uptime in hours. * Modify `RecordRuntimeStats` in `src/common/app_metrics.cpp` to call `RecordUptime`. * Update `src/common/appmain.cpp` to call `RecordUptime` in the `app_run` function. * Add a new line to display the uptime in hours in the statistics menu in `src/gui/screen_menu_statistics.cpp`. * Add a new member variable to store the uptime and a new method to update the uptime in `src/gui/screen_menu_statistics.hpp`.
CZDanol
requested changes
Jan 10, 2025
| @@ -133,6 +133,8 @@ void buddy::metrics::RecordRuntimeStats() { | |||
|
|
|||
| METRIC_DEF(heap, "heap", METRIC_VALUE_CUSTOM, 503, METRIC_HANDLER_ENABLE_ALL); | |||
| metric_record_custom(&heap, " free=%zui,total=%zui", xPortGetFreeHeapSize(), static_cast<size_t>(heap_total_size)); | |||
|
|
|||
| RecordUptime(); | |||
There was a problem hiding this comment.
- Adding a metric is a separate thing from adding a statistics item, it should be in a separate commit
- We use
snake_casefor functions as per https://github.com/prusa3d/Prusa-Firmware-Buddy/blob/master/doc/contributing.md - No other metric in the RecordRuntimeStats has a dedicated record function - don't see why this one should
| @@ -233,6 +233,7 @@ void app_run(void) { | |||
| loop(); | |||
| } | |||
| marlin_server::loop(); | |||
| buddy::metrics::RecordUptime(); // Pa465 | |||
There was a problem hiding this comment.
- What does this comment stand for?
- Why do we need to call this function from two places?
| @@ -4,11 +4,14 @@ | |||
| #include "screen_menu_statistics.hpp" | |||
| #include "DialogMoveZ.hpp" | |||
| #include "img_resources.hpp" | |||
| #include "app_metrics.h" | |||
There was a problem hiding this comment.
Why are we including app_metrics here?
| void ScreenMenuStatistics::updateUptime() { | ||
| char buffer[32]; | ||
| snprintf(buffer, sizeof(buffer), "Uptime: %lu hours", ticks_s() / 3600); | ||
| uptime_label.SetText(buffer); |
There was a problem hiding this comment.
- SetText takes
string_view_utf8, you're providing char[] - We have separate fields for "text" and for "value" - putting everything together is not acceptable
- This code does not take translations into account
- The label should at least also show minutes as well
|
|
||
| ScreenMenuStatistics::ScreenMenuStatistics() | ||
| : ScreenMenuStatistics__(_(label)) { | ||
| EnableLongHoldScreenAction(); | ||
| header.SetIcon(&img::info_16x16); | ||
| AddItem(&uptime_label); |
There was a problem hiding this comment.
This is not a valid code for adding menu items in the menu.
- The menu must be added to the
ScreenMenutemplate as a parameter - AddItem function is nowhere in the firmware
- Use
MenuItemAutoUpdatingLabelfor an automatically updating label
CZDanol
reviewed
Jan 10, 2025
| @@ -352,4 +354,9 @@ void buddy::metrics::record_dwarf_internal_temperatures() { | |||
| } | |||
| } | |||
| } | |||
|
|
|||
| void buddy::metrics::RecordUptime() { | |||
| METRIC_DEF(uptime, "uptime", METRIC_VALUE_INTEGER, 1000, METRIC_HANDLER_ENABLE_ALL); | |||
There was a problem hiding this comment.
The "1000" there means that we would be sending the metric every second. I would say that is a bit wasteful.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes #4392
Add uptime to the statistics menu.
RecordUptimeinsrc/common/app_metrics.cppto record the uptime in hours.RecordRuntimeStatsinsrc/common/app_metrics.cppto callRecordUptime.src/common/appmain.cppto callRecordUptimein theapp_runfunction.src/gui/screen_menu_statistics.cpp.src/gui/screen_menu_statistics.hpp.