-
Notifications
You must be signed in to change notification settings - Fork 247
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
GUI: Fix footer update after selecting None #4345
base: master
Are you sure you want to change the base?
Conversation
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
src/gui/footer/footer_line.cpp
Outdated
@@ -51,6 +51,8 @@ bool FooterLine::Create(footer::Item item_id, size_t index) { | |||
|
|||
if (result) { | |||
positionWindows(); | |||
} else if (item_id == footer::Item::none) { | |||
Erase(index); |
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 doesn't seem to be a correct fix. The result is set to true on any change, which should call positionWindows. That is, even on change to/from none. This thing might possibly work, but it's a bit random, plus it will call positionWindows each time (inside Erase), suboptimally.
By quickly looking at the code, the proper/elegant fix would probably be to add FooterItemMappingRec<std::monostate, Item::none>
to FooterItemMappings
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.
Trying to add FooterItemMappingRec as suggested doesn't work due to compiler error:
error: static assertion failed: T must occur exactly once in alternatives [build] 1115 | static_assert(__detail::__variant::__exactly_once<_Tp, _Types...>,
.
As far as I could see, Erase doesn't call positionWindows and it only is erased if result is false, which happens if the item isn't registered (which is only the case for footer::item::none).
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.
I'm now using unregister directly and also moved it above the FooterItemsMapping function, which should be a more proper way to do it. If you have any ideas how to make the FooterItemMappingRec working with std::monostate, please let me know :)
069bf61
to
4202f5a
Compare
4202f5a
to
acade0a
Compare
@@ -41,6 +41,9 @@ bool FooterLine::Create(footer::Item item_id, size_t index) { | |||
|
|||
if (item_id >= footer::Item::_count) { | |||
item_id = footer::Item::none; | |||
} else if (item_id == footer::Item::none) { | |||
unregister(index); |
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 does not call positionWindows, which it probably should in some cases (at least according to the previous code).
@@ -41,6 +41,9 @@ bool FooterLine::Create(footer::Item item_id, size_t index) { | |||
|
|||
if (item_id >= footer::Item::_count) { | |||
item_id = footer::Item::none; | |||
} else if (item_id == footer::Item::none) { | |||
unregister(index); | |||
return false; |
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.
It is rather debatable whether this should call true
or false
. From the previous code, it rather seems like the function should return true
if an item is changed (and in that case, and only in that case, it should call reposition). This code does not follow that logic.
But looking around, it seems like the return value is not used anywhere, so it would perhaps be best to just turn it into a procedure.
This fixes issue #4166.
After selecting None in the Footer settings menu, the existing footer item isn't erased, thus stays active although it has been disabled.