Skip to content

fix uninitialized cutting power warning in M3-M5.cpp#26091

Merged
thinkyhead merged 3 commits intoMarlinFirmware:bugfix-2.1.xfrom
alx3dev:bugfix-2.1.x
Aug 7, 2023
Merged

fix uninitialized cutting power warning in M3-M5.cpp#26091
thinkyhead merged 3 commits intoMarlinFirmware:bugfix-2.1.xfrom
alx3dev:bugfix-2.1.x

Conversation

@alx3dev
Copy link
Contributor

@alx3dev alx3dev commented Jul 18, 2023

Description

When compiling Marlin for Creality v4.2.7 board and LASER_FEATURE is enabled, there is warning saying that u may be used uninitialized:

Marlin/src/gcode/control/M3-M5.cpp: In function 'GcodeSuite::M3_M4(bool)::<lambda()>':
Marlin/src/gcode/control/M3-M5.cpp:103:41: warning: 'u' may be used uninitialized in this function [-Wmaybe-uninitialized]
  103 |     cutter.menuPower = cutter.unitPower = u;

This should be independent of board, and can be suppressed with adding 0.0 value to u variable.

change line 95:
float u; to float u = 0.0f

Requirements

Require LASER_FEATURE enabled, should happen on all boards, but checked on STM32F1.

Benefits

Remove warning that say u may be run uninitialized in function.

Configurations

configuration.zip

alx3dev and others added 2 commits July 18, 2023 21:32
add `u = 0.0f` to suppress warning that `u` may be used uninitialized in `auto get_s_power`
@thinkyhead
Copy link
Member

It might be that the intent was to not modify cutter.unitPower outside of M3 S/M4 S when cutter.cutter_mode != CUTTER_MODE_STANDARD so I went ahead and reverted that part of the commit 55daab0a from last April. If @descipher is still around then maybe he can confirm that this is the correct fix.

Another equivalent is…

auto get_s_power = [] {
  float u = cutter.unitPower;
  if (parser.seenval('S')) {
    const float v = parser.value_float();
    u = TERN(LASER_POWER_TRAP, v, cutter.power_to_range(v));
  }
  else if (cutter.cutter_mode == CUTTER_MODE_STANDARD)
    u = cutter.cpwr_to_upwr(SPEED_POWER_STARTUP);

  cutter.menuPower = cutter.unitPower = u;

  // PWM not implied, power converted to OCR from unit definition and on/off if not PWM.
  cutter.power = TERN(SPINDLE_LASER_USE_PWM, cutter.upower_to_ocr(u), u > 0 ? 255 : 0);
};

… but this results in cutter.menuPower always being set equal to cutter.unitPower, and I'm not sure that is always appropriate.

@descipher
Copy link
Contributor

I will review it and post what I see.

@descipher
Copy link
Contributor

@thinkyhead, Thanks for the ping on the change, you are correct the intent was to keep the STANDARD_MODE as backward compatible for older use cases. The one exception is when SYNC mode is enabled which allows a user to override it immediately appliying the PWM value within the planner and not waiting for the M3 move to finnish. The menu code should update the active display value, however its very slow and clunky with the 8 bit implementation. The menuPower is a quirky thing where the previous code needed some separation in order to display the chosen power units e.g. 15000 RPM, 100% etc. could be changed to work better but those legacy display functions are not very co-operative in some cases.

@thinkyhead thinkyhead merged commit 709def5 into MarlinFirmware:bugfix-2.1.x Aug 7, 2023
mikezs added a commit to mikezs/Marlin that referenced this pull request Aug 15, 2023
* bugfix-2.1.x: (427 commits)
  [cron] Bump distribution date (2023-08-14)
  🔧 Configurable SD card retry/timeout (MarlinFirmware#25340)
  [cron] Bump distribution date (2023-08-08)
  🐛 Fix MKS Robin Mini servo timer (MarlinFirmware#26150)
  🚸 Adjust ColorUI chamber bmp (MarlinFirmware#26149)
  🚸 UI Sound off/on with M300 E<0|1> (MarlinFirmware#26142)
  🐛 Fix UBL probe_entire_mesh skips points (MarlinFirmware#26141)
  🔨 Fix USB FD env names (MarlinFirmware#26131)
  🩹 PROBING_TOOL followup (MarlinFirmware#26122)
  🔧 Clarify WIFISUPPORT (MarlinFirmware#26097)
  🩹 Fix M3 `uninitialized` warning (MarlinFirmware#26091)
  🚸 FT_MOTION menu updates (MarlinFirmware#26083)
  [cron] Bump distribution date (2023-08-07)
  🚸 BD Sensor Z axis stop height (MarlinFirmware#26015)
  ⚡️ SAMD21 LCD uses HW SPI with media (MarlinFirmware#26012)
  🚸 Update LCD Manual Leveling display (MarlinFirmware#26088)
  📝 STM32G0B0 SKR Mini E3 V3.0 / Manta M4P (MarlinFirmware#26087)
  📝 Update a config comment
  [cron] Bump distribution date (2023-08-06)
  ✨ MM-JOKER (ESP32) board (MarlinFirmware#25897)
  ...

# Conflicts:
#	.github/workflows/bump-date.yml
#	.github/workflows/clean-closed.yml
#	.github/workflows/test-builds.yml
#	Marlin/Configuration.h
#	Marlin/Configuration_adv.h
#	Marlin/src/pins/pins.h
Pragma8123 pushed a commit to Pragma8123/Bender that referenced this pull request Oct 24, 2023
EvilGremlin pushed a commit to EvilGremlin/Marlin that referenced this pull request Oct 26, 2023
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.

3 participants