Skip to content
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

Various small fixes/tweaks #26502

Conversation

classicrocker883
Copy link
Contributor

@classicrocker883 classicrocker883 commented Dec 6, 2023

Description

Most file changes remove HAS_STATUS_MESSAGE swap w/HAS_DISPLAY
Also, 2nd most are changes to whitespace, and comments.

  • remove whitespace/fix wording, tweak spacing
  • add hyphen (-) to "Gcode" -- (follow up to "📝 GCode => G-Code") i believe done in another PR
  • remove unused INLINE_USART_IRQ done in another PR
  • optimize code - add MMS_SCALED to feedrate_mm_s + simplified the PID Plot code in proui/dwin.cpp so it's easier to read
  • add a Default-No Probe/No Mesh test to buildroot/tests/STM32F103RE_creality
  • Removed COLOR_AQUA because it is unused, and also the same as COLOR_CYAN
  • add HAS_TRAMMING_WIZARD
  • set_status_and_level(..., level=0) from Enabled 2 mesh viewers #26181 done in another PR

    • this should be looked at if this change is needed vs change in other PR(see Note)
    • same thing change for M48.cpp
      without level=0, we can just add , 0 like so set_status_and_level(...code.., 0)??
  • extrude_length => purge_length to match others done in another PR
  • dtostrf => p/w_float_t -- M114.cpp|M48.cpp

Update

follow up to "[BUG] DWIN ProUI: error: 'PROBE_OFFSET_ZMIN' was not declared in this scope #26505"
in ProUI, created _OFFSET_ZMIN/MAX like in is in CrealityUI, made it so it = PROBE_OFFSET_ZMIN/MAX if HAS_BED_PROBE/defined, otherwise = +/-20

  • optimize #define/#include in JyersUI and CrealityUI as well

  • use PROBE_SELECTED to enabled PROBE_OFFSET_ZMIN|MAX (because --see below--`)

  • simplified Print Progress (_printtime, _remain_time, _percent_done) in ProUI
    may need to adjust layout position on the UI

    • made use of duration_t -- toDigital, and pcttostrpctrj
    • benefits: less bytes, also adds Days to print time elapsed/remaining
  • update bedlevel_tools.cpp, utilize unused functions

  • add function to zeroPoint (reset single mesh x/y coordinate)

  • update trammingwizard:
    shows correct screen while inuse - rather than user being able to still navigate menu
    exits the wizard - displays within tolerance - if values are 0.00 as fail-safe, otherwise gives false reading.
    show's Lower/Raise in green/red.

  • move dwinIconShow to respected header

  • add to ESDiag: filament sensor "PRESENT" (green) / "Runout Detected" (red)

    • rename ES_REPORT => ES_REPORTS to not be confused with previously defined in endstops.cpp
  • remove/replace dwinPopupConfirm w/ dwinPopupContinue

  • typo in module/probe.cpp Probe::run_z_probe(). "-float" to "- float"

  • swap const char * with PGM_P (which is defined as const char *)

  • Tweak CASELIGHT_USES_BRIGHTNESS in ProUI, separate function that sets it into "apply" and "live"

  • fix tramming wizard, so now:

When x = 0 and y = 0, p will be calculated as p = 0 + 2 * 0 = 0.
When x = 0 and y = 1, p will be calculated as p = 1 + 2 * 0 = 1.
When x = 1 and y = 0, p will be calculated as p = 0 + 2 * 1 = 2.
When x = 1 and y = 1, p will be calculated as p = 1 + 2 * 1 = 3.

Note

I'm not so sure about this one

  • in pinsDebug_list.h I added Maple support which have their SDIO defined as BOARD_SDIO_*
 #if PIN_EXISTS(SDIO_D0)
   REPORT_NAME_DIGITAL(__LINE__, SDIO_D0_PIN)
+#elif defined(BOARD_SDIO_D0)
+  REPORT_NAME_DIGITAL(__LINE__, BOARD_SDIO_D0)
 #endif

in Marlin\src\inc\Conditionals_post.h
you will see

#if !HAS_LEVELING || ANY(MESH_BED_LEVELING, AUTO_BED_LEVELING_UBL)
  #undef PROBE_MANUALLY
#endif
#if ANY(HAS_BED_PROBE, PROBE_MANUALLY, MESH_BED_LEVELING)
  #define PROBE_SELECTED 1
#endif

removed HAS_ONESTEP_LEVELING, replaced with HAS_AUTOLEVEL because
(previously...)

#if DISABLED(PROBE_MANUALLY) && ANY(AUTO_BED_LEVELING_BILINEAR, AUTO_BED_LEVELING_LINEAR, AUTO_BED_LEVELING_3POINT, AUTO_BED_LEVELING_UBL)
  #define HAS_ONESTEP_LEVELING 1
#endif

this code is the same as HAS_AUTOLEVEL, as you can see here:

#if ANY(HAS_ABL_NOT_UBL, AUTO_BED_LEVELING_UBL)
  #define HAS_ABL_OR_UBL 1
  #if DISABLED(PROBE_MANUALLY)
    #define HAS_AUTOLEVEL 1
  #endif
#endif

Requirements

Benefits

Configurations

Related Issues

if anyone wishes to take another look, these are what I suggest:
see if buildroot/tests/STM32F103RE_creality needs anything

and also...

in JyersUI/dwin.cpp
MAX_Z_OFFSET seemed out of place without MIN_Z_OFFSET
basically these values can be used even without HAS_Z_OFFSET_ITEM (BABYSTEPPING), it would cause an error being undefined for some Configurations

-#if HAS_ZOFFSET_ITEM
-  #define MAX_Z_OFFSET 9.99
-  #if HAS_BED_PROBE
-    #define MIN_Z_OFFSET -9.99
-  #else
-    #define MIN_Z_OFFSET -1
-  #endif
-#endif

+#if HAS_ZOFFSET_ITEM && HAS_BED_PROBE
+  #define MAX_Z_OFFSET  20
+  #define MIN_Z_OFFSET -20
+#else
+  #define MAX_Z_OFFSET  3
+  #define MIN_Z_OFFSET -3
+#endif

remove whitespace, add hyphen (-)
remove unused `INLINE_USART_IRQ`
optimize code
have `HAS_Z_OFFSET_ITEM` defined in one file
undef PROBE_OFFSET_ZMIN when ProUI is disabled
docs/Cutter.md Outdated Show resolved Hide resolved
@thisiskeithb
Copy link
Member

made it so to #undef PROBE_OFFSET_ZMIN/MAX when DWIN_LCD_PROUI is disabled. because this value is used when editing Z Offset. even without a probe or leveling, and babystepping is enabled

Sounds like these Ender-3 V2 UIs need to decouple from requiring/assuming a probe is installed instead of this workaround.

@classicrocker883
Copy link
Contributor Author

classicrocker883 commented Dec 8, 2023

Sounds like these Ender-3 V2 UIs need to decouple from requiring/assuming a probe is installed instead of this workaround.

it uses that probe offset value regardless of one or not, using it for changing the Z-offset.

how about using a TERN in the function?

- setPFloatOnClick(PROBE_OFFSET_ZMIN, PROBE_OFFSET_ZMAX, 2, applyZOffset, liveZOffset);
+ setPFloatOnClick(TERN(PROBE_OFFSET_ZMIN, 20), TERN(PROBE_OFFSET_ZMAX, 20), 2, applyZOffset, liveZOffset);

or completely omit this #undef since the other PROBE_OFFSET_ arent included?

or we could add somewhere

#ifndef PROBE_OFFSET_ZMIN
  #define PROBE_OFFSET_ZMIN 20
#endif
#ifndef PROBE_OFFSET_ZMAX
  #define PROBE_OFFSET_ZMAX 20
#endif

"#if DISABLED(PROBE_MANUALLY) && ANY(AUTO_BED_LEVELING_BILINEAR, AUTO_BED_LEVELING_LINEAR, AUTO_BED_LEVELING_3POINT, AUTO_BED_LEVELING_UBL)" is literally HAS_AUTOLEVEL
@@ -1139,7 +1131,7 @@ void popupWindowHome(const bool parking/*=false*/) {
}
}

#if HAS_ONESTEP_LEVELING
#if HAS_AUTOLEVEL
Copy link
Member

@thinkyhead thinkyhead Dec 8, 2023

Choose a reason for hiding this comment

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

This includes UBL but unfortunately UBL is rarely a one step leveling process. It usually requires an auto leveling step followed by a manual leveling step. So some additional checking must be done to ensure that UBL is actually one-step (i.e., it can reach all mesh points with the probe) or additional UI must be developed to guide users through the following steps.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

so then it makes sense to be HAS_AUTOLEVEL?
as far as I understand it, UBL doesnt have to be followed by manual leveling. the auto level option usually probes all the points, and any points not able to reach, it fills in the unprobed values based on the surrounding probed points, and in my experience with great accuracy.

for Bilinear or UBL, its usually the default, and one and done kind of thing. and if they wanted to check the points or do it manually they have that option as well.

Copy link
Member

Choose a reason for hiding this comment

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

UBL only fills in the un-probed values when commanded to do so by a call to G29 P3. I don't think this old Creality screen implementation does that.

classicrocker883 and others added 26 commits February 19, 2024 10:24
@thinkyhead
Copy link
Member

It is not possible to review all these changes and accept them all in one PR. You should cherry-pick parts of this large PR into smaller pieces with changes limited to specific areas of the code, or only addressing specific things, and submit those separately as smaller PRs.

@thinkyhead thinkyhead closed this May 21, 2024
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.

4 participants