Skip to content

馃悰 Prevent MString infinite recursion - #26037

Merged
thinkyhead merged 1 commit into
MarlinFirmware:bugfix-2.1.xfrom
thinkyhead:bf2_string_fix_PR
Jul 1, 2023
Merged

馃悰 Prevent MString infinite recursion#26037
thinkyhead merged 1 commit into
MarlinFirmware:bugfix-2.1.xfrom
thinkyhead:bf2_string_fix_PR

Conversation

@thinkyhead

@thinkyhead thinkyhead commented Jul 1, 2023

Copy link
Copy Markdown
Member

Followup to #24390

  • Add a single typename T template for MString::append to handle unspecified types (by appending '?' to the string). This removes the infinite recursion warning for the variadic template MString::append(T, ...) and may avoid actual infinite recursion if unhandled types are passed in MString::append(T, ...).

  • Add a standard handler for explicit int8_t& so this type will be handled as a small integer instead of being handled as a char or deferring to the fallback typename T.

  • Suppress warnings about deprecated sprintf on native platforms.

@thinkyhead
thinkyhead merged commit 4985aca into MarlinFirmware:bugfix-2.1.x Jul 1, 2023
@thinkyhead
thinkyhead deleted the bf2_string_fix_PR branch July 1, 2023 09:33
@thisiskeithb

thisiskeithb commented Jul 1, 2023

Copy link
Copy Markdown
Contributor

The bootlooping has stopped with this PR, but now temperatures are not reported correctly over serial:

OctoPrint log:

Send: M140 S70

Recv: //action:notification Bed Heating...
Recv: ok P31 B31
Recv:  T? /0.00 B? /70.00 @:0 B@:127
Recv:  T? /0.00 B? /70.00 @:0 B@:127

Send: M104 S210

Recv: //action:notification E1 Heating...
Recv: ok P31 B31
Recv:  T? /210.00 B? /70.00 @:127 B@:127
Recv:  T? /210.00 B? /70.00 @:127 B@:127
Recv:  T? /210.00 B? /70.00 @:127 B@:127
[鈥

I also can't connect to the motherboard with a BTT TFT (connected over serial) and it always says "No printer attached!".

@ellensp

ellensp commented Jul 1, 2023

Copy link
Copy Markdown
Contributor

No change on eeprom issue.

stock RAMPS with EEPROM_SETTINGS enabled (and the sim)

M500

echo:Settings Stored (617 bytes; crc 59424)
Error:EEPROM datasize error.
ok

@rondlh

rondlh commented Jul 1, 2023

Copy link
Copy Markdown
Contributor

The bootlooping has stopped with this PR, but now temperatures are not reported correctly over serial:
OctoPrint log:

I also can't connect to the motherboard with a BTT TFT (connected over serial) and it always says "No printer attached!".

That is because the temperature reporting is completely broken/missing after #24390 and #26037. The BTT TFT uses the temperature reporting to confirm it's connected.

@thisiskeithb

Copy link
Copy Markdown
Contributor

No change on eeprom issue.

I don't see an error on an SKR V3 (with onboard EEPROM):

>>> M502
SENDING:M502
//action:notification Hardcoded Default Settings Loaded
echo:Hardcoded Default Settings Loaded
ok P31 B31

>>> M500
SENDING:M500
echo:Settings Stored (746 bytes; crc 18007)
//action:notification Settings Stored
//action:notification Settings Stored
ok P31 B31

Also no error with an SKR V2 (emulated EEPROM via flash):

>>> M502
SENDING:M502
//action:notification Hardcoded Default Settings Loaded
echo:Hardcoded Default Settings Loaded

>>> M500
SENDING:M500
echo:Settings Stored (891 bytes; crc 43788)
//action:notification Settings Stored
//action:notification Settings Stored

@RV-from-be

RV-from-be commented Jul 1, 2023

Copy link
Copy Markdown

@thisiskeithb
By modifying temperature.cpp:

#define SFP _MIN(SERIAL_FLOAT_PRECISION, 2)

to:

    #ifdef SERIAL_FLOAT_PRECISION
      #define SFP _MIN(SERIAL_FLOAT_PRECISION, 2)
    #else
      #define SFP 2
    #endif

and:

s += TS(':', p_float_t(c, SFP));

to:

    s += F(":"); s += p_float_t(c, SFP);

This should resolve the temperature reporting issue and possibly the connection with the BTT TFT.

@thisiskeithb

thisiskeithb commented Jul 1, 2023

Copy link
Copy Markdown
Contributor
 s += F(":"); s += p_float_t(c, SFP);

This should resolve the temperature reporting issue and possibly the connection with the BTT TFT.

Indeed, that does fix the temperature reporting & BTT TFT connectivity issues.

Andy-Big pushed a commit to Andy-Big/Marlin_FB_Reborn that referenced this pull request Jul 17, 2023
Andy-Big pushed a commit to Andy-Big/Marlin_FB_Reborn that referenced this pull request Jul 20, 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.

[BUG] Reboot on USB/Serial Connect and EEPROM Errors

5 participants