gui: "Settings->System->Load settings from File": Provide Feedback #3769
Conversation
|
Good morning Agreed it would be nice to have some feedback coming from this menu item. It's always been this very out of the way „geek“ access. Nevertheless, the current solution seems both a bit overkill (a complete new screen) and not really looking nice. Do you think a simple message box (possibly with slightly less detailed output, maybe just success/failure) would suffice? An inspiration can be taken here: https://github.com/prusa3d/Prusa-Firmware-Buddy/blob/master/src/gui/screen_menu_connect.cpp#L35 |
bkerler
force-pushed
the
load_settings_feedback
branch
from
25b8538
to
d80f1fc
Compare
|
@vorner Couldn't agree more. I tried a more minimalistic approach based on your code recommendation, I hope you like it :) |
bkerler
force-pushed
the
load_settings_feedback
branch
from
d80f1fc
to
4e4a242
Compare
bkerler
force-pushed
the
load_settings_feedback
branch
from
4e4a242
to
929d43c
Compare
bkerler
force-pushed
the
load_settings_feedback
branch
from
929d43c
to
a331bcc
Compare
bkerler
force-pushed
the
load_settings_feedback
branch
from
a331bcc
to
8274b82
Compare
src/common/str_utils.cpp
Outdated
| } | ||
|
|
||
| char b; | ||
| str.copyToRAM(&b, 1); |
There was a problem hiding this comment.
Uff, haven't noticed this before. copyToRam is quite heavy for this, I'd suggest making getbyte public instead.
There was a problem hiding this comment.
it's now lightweighter :)
bkerler
force-pushed
the
load_settings_feedback
branch
from
8274b82
to
9892682
Compare
src/common/str_utils.cpp
Outdated
| return; | ||
| } | ||
|
|
||
| char b = str.getUtf8Char(); |
There was a problem hiding this comment.
getUtf8Char does decoding and returns a full unicode. You cannot use it here. You have to use getbyte
There was a problem hiding this comment.
getbyte was private, so I added a public getbyte function
src/gui/MItem_menus.cpp
Outdated
| StringBuilder msg_builder(msg); | ||
| msg_builder.append_string_view(_("\nLoading settings finished.\n\n")); | ||
|
|
||
| bool network_settings_loaded = netdev_load_ini_to_eeprom(); |
There was a problem hiding this comment.
Nitpick: This can be made const bool
bkerler
force-pushed
the
load_settings_feedback
branch
from
9892682
to
a45f122
Compare
|
Hmm, I've noticed the unit tests don't build now. Can you have a look? This one should be easy. |
bkerler
force-pushed
the
load_settings_feedback
branch
from
a45f122
to
474a274
Compare
|
@vorner Tests build fine now. Everything looks perfect. |
This PR will add user feedback to the function "Settings->System->Load settings from File".
There was no feedback given and users cannot see if their "prusa_printer_settings.ini" failed or not.