[BUG] Build errors on STM32F103RC_btt when SEGMENT_LEVELED_MOVES disabled

[doctorthompson]

Bug Description

A slew of overloading and narrowing errors are produced while compiling abl.cpp when SEGMENT_LEVELED_MOVES is disabled in Configuration.h. Build output is included in the ZIP file with my config files. Target board is STM32F103RC_btt_512K_USB, though similar errors are produced when building for both the non-USB and non-512K variety as well.

My Configurations

Configs_and_Build_Logs.zip

Steps to Reproduce

1. Comment out SEGMENT_LEVELED_MOVES and LEVELED_SEGMENT_LENGTH in Configuration.h
2. Build for STM32F103RC_btt_512K_USB

Expected behavior: Build succeeds and produces a valid firmware binary.

Actual behavior: Several screens worth of C++ errors pertaining to overloading and narrowing are output and the build fails.

Additional Information

Branch used: bugfix-2.0.x as of 2020-09-18, commit 648269e

With the two defines uncommented the build succeeds, albeit with a few odd warnings. These have been included at the end of the build output file in the ZIP.

[ellensp]

Issue is not board dependent
With standard provided ramps config it just needs AUTO_BED_LEVELING_BILINEAR and a probe setup (I used FIX_MOUNTED_PROBE) with SEGMENT_LEVELED_MOVES disabled and it gives this error

only has issues with AUTO_BED_LEVELING_BILINEAR other leveling methods works fine.

[shitcreek]

Removing the & operator in front of &scaled_fr_mm_s in abl.h resolves the overloading error and compiles successfully (with AUTO_BED_LEVELING_BILINEAR enabled and SEGMENT_LEVELED_MOVES disabled). Narrowing conversion warnings still come up.

Changed from:

#if IS_CARTESIAN && DISABLED(SEGMENT_LEVELED_MOVES)
  void bilinear_line_to_destination(const feedRate_t &scaled_fr_mm_s, uint16_t x_splits=0xFFFF, uint16_t y_splits=0xFFFF);
#endif

To:

#if IS_CARTESIAN && DISABLED(SEGMENT_LEVELED_MOVES)
  void bilinear_line_to_destination(const feedRate_t scaled_fr_mm_s, uint16_t x_splits=0xFFFF, uint16_t y_splits=0xFFFF);
#endif

[github-actions]

This issue has had no activity in the last 30 days. Please add a reply if you want to keep this issue active, otherwise it will be automatically closed within 7 days.

[XBrav]

I can confirm the above fix by @shitcreek bypasses the build errors on 2.0.7.1. Do we know exactly what the impact is of removing the pointer?

[doctorthompson]

Well, it's a reference not a pointer, but looking at the typedef it looks like it's just a simple float.

If we were just running on run-of-the-mill CPUs I'd say passing it by value probably shouldn't be much of an issue.

However, I'm not familiar enough with all the microcontrollers Marlin supports to be able to say whether passing a float by value (possibly many times) is going to result in memory or stack issues on some of the more constrained hardware.

The implications are: Without the ampersand it will need to allocate whatever sizeof(float) is on the underlying platform for each call.

It certainly shouldn't be an issue on any of the beefier boards.

[thinkyhead]

It's preferred to pass float as a const reference for performance on AVR because on that architecture a float is 4 bytes while a reference or pointer is only 2 bytes. The feedRate_t is a simple alias for float. So, adding the ampersand to the function definition where it's missing should keep the stack smaller on AVR.

I have been considering adding a new type, if needed, to pass float in the most optimal way according to the platform. The platforms declare integer types with least_ and fast_ in their names, so I might follow that naming pattern.

[sjasonsmith]

This appears to be resolved. After updating the original configs enough to build against bugfix the only warnings came from external libraries. Closing.

[github-actions]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.