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

[BUG] compile ia_creality fails #25429

Closed
1 task done
rmbq opened this issue Feb 25, 2023 · 15 comments
Closed
1 task done

[BUG] compile ia_creality fails #25429

rmbq opened this issue Feb 25, 2023 · 15 comments

Comments

@rmbq
Copy link
Contributor

rmbq commented Feb 25, 2023

Did you test the latest bugfix-2.1.x code?

Yes, and the problem still exists.

Bug Description

after f0cc8bd i can't compile with
#define DGUS_LCD_UI IA_CREALITY
and
#define MOTHERBOARD BOARD_CREALITY_V453

Errors:

Marlin\src\lcd\extui\ia_creality\creality_extui.cpp:113:84: error: call of overloaded 'RTS_SndData(int, int)' is ambiguous
  113 |     rtscheck.RTS_SndData(TERN0(HAS_MULTI_HOTEND, getActualTemp_celsius(H1)), e2Temp);
      |                                                                                    ^
In file included from Marlin\src\lcd\extui\ia_creality\creality_extui.cpp:37:
Marlin\src\lcd\extui\ia_creality\creality_extui.h:234:12: note: candidate: 'void ExtUI::RTSSHOW::RTS_SndData(const String&, uint32_t, uint8_t)'
  234 |       void RTS_SndData(const String &, uint32_t, uint8_t=VarAddr_W);
      |            ^~~~~~~~~~~
*** [.pio\build\STM32F103RC_creality\src\src\lcd\extui\ia_creality\FileNavigator.cpp.o] Error 1
Marlin\src\lcd\extui\ia_creality\creality_extui.h:235:12: note: candidate: 'void ExtUI::RTSSHOW::RTS_SndData(const char*, uint32_t, uint8_t)'
  235 |       void RTS_SndData(const char[], uint32_t, uint8_t=VarAddr_W);
      |            ^~~~~~~~~~~
Marlin\src\lcd\extui\ia_creality\creality_extui.h:236:12: note: candidate: 'void ExtUI::RTSSHOW::RTS_SndData(char, uint32_t, uint8_t)'
  236 |       void RTS_SndData(char, uint32_t, uint8_t=VarAddr_W);
      |            ^~~~~~~~~~~
Marlin\src\lcd\extui\ia_creality\creality_extui.h:237:12: note: candidate: 'void ExtUI::RTSSHOW::RTS_SndData(uint8_t*, uint32_t, uint8_t)'
  237 |       void RTS_SndData(uint8_t*, uint32_t, uint8_t=VarAddr_W);
      |            ^~~~~~~~~~~
Marlin\src\lcd\extui\ia_creality\creality_extui.h:238:12: note: candidate: 'void ExtUI::RTSSHOW::RTS_SndData(int16_t, uint32_t, uint8_t)'
  238 |       void RTS_SndData(int16_t, uint32_t, uint8_t=VarAddr_W);
      |            ^~~~~~~~~~~
Marlin\src\lcd\extui\ia_creality\creality_extui.h:239:12: note: candidate: 'void ExtUI::RTSSHOW::RTS_SndData(float, uint32_t, uint8_t)'
  239 |       void RTS_SndData(float, uint32_t, uint8_t=VarAddr_W);
      |            ^~~~~~~~~~~
Marlin\src\lcd\extui\ia_creality\creality_extui.h:240:12: note: candidate: 'void ExtUI::RTSSHOW::RTS_SndData(uint16_t, uint32_t, uint8_t)'
  240 |       void RTS_SndData(uint16_t, uint32_t, uint8_t=VarAddr_W);
      |            ^~~~~~~~~~~
Marlin\src\lcd\extui\ia_creality\creality_extui.h:241:12: note: candidate: 'void ExtUI::RTSSHOW::RTS_SndData(int32_t, uint32_t, uint8_t)'
      |            ^~~~~~~~~~~
Marlin\src\lcd\extui\ia_creality\creality_extui.h:242:12: note: candidate: 'void ExtUI::RTSSHOW::RTS_SndData(uint32_t, uint32_t, uint8_t)'
  242 |       void RTS_SndData(uint32_t, uint32_t, uint8_t=VarAddr_W);
      |            ^~~~~~~~~~~
compilation terminated due to -fmax-errors=5.
*** [.pio\build\STM32F103RC_creality\src\src\lcd\extui\ia_creality\creality_extui.cpp.o] Error 1

Bug Timeline

since f0cc8bd

Expected behavior

compile

Actual behavior

error while compile

Steps to Reproduce

Compile with
#define DGUS_LCD_UI IA_CREALITY
and
#define MOTHERBOARD BOARD_CREALITY_V453

Version of Marlin Firmware

last bugfix 2.1.x

Printer model

creality cr 10 smart

Electronics

stock

Add-ons

none

Bed Leveling

None

Your Slicer

None

Host Software

None

Don't forget to include

  • A ZIP file containing your Configuration.h and Configuration_adv.h.

Additional information & file uploads

Configuration Files

@bob-anthony
Copy link
Contributor

This environment compiles fine using the official InsanityAutomation repo. I can't imagine why code hacked in without the original developer's approval or support wouldn't operate as expected.

@ellensp
Copy link
Contributor

ellensp commented Feb 26, 2023

Interestingly on mega2560 (what this display was designed for) It compiles without issues.

@ellensp
Copy link
Contributor

ellensp commented Feb 26, 2023

@rmbq Please test this PR

@rmbq
Copy link
Contributor Author

rmbq commented Feb 26, 2023

@ellensp now it compiles but marlin doesn't communicate correctly with the screen.
using this line
char (*__kaboom)[sizeof( int )] = 1;
from here
shows sizeof(int) is 4. With f0cc8bd all unsigned int and int are converted to uint16 or int16 and not uint32 or int32

@ellensp
Copy link
Contributor

ellensp commented Feb 26, 2023

yes, but on avr they are 2 bytes, not 4. Just changing them to 32 is not the answer. That is going backwards. Need to check if any actually exceed 16bit values..

@rmbq
Copy link
Contributor Author

rmbq commented Feb 26, 2023

Probably the screen also needs int32 to works. Maybe we need the change the screen firmware to use 16 instead of 32.

@InsanityAutomation
Copy link
Contributor

InsanityAutomation commented Feb 26, 2023

Be careful making changes with overloaded methods. This situation demonstrated fairly quickly why I didn't put this upstream. Try reverting the breaking changes, as I doubt this was the only UI broken.

@thinkyhead
Copy link
Member

thinkyhead commented Feb 27, 2023

…why I didn't put this upstream…

@InsanityAutomation

I could use your help sorting out the Resistive Touch Screen code, as we're getting another one in support of the Ender-5 S1, and it would be awesome to have all the RTS screen common code in a root class, and then make sub-classes for the different variants. Plus, the Ender-5 S1 user interface is less than ideal, including not yet supporting folders on SD card.

As for the ambiguity of the overloaded methods, we can probably get those sorted by going through all the calls to RTS_SndData and just making sure we know the types in use and eliminating any lingering amibiguities, especially related to signed/unsigned and 8/16/32 bit. It's always a bit annoying to deal with C++ overloading in this area. Where needed, we can get rid of overloading and give these methods names that include the data size, such as RTS_SndData16 and RTS_SndData32, etc.

@thinkyhead
Copy link
Member

@rmbq — Thanks for the report. I'll have this patched forthwith!

@thinkyhead
Copy link
Member

@rmbq — There are a number of other compile errors with those configurations. What version of Marlin are you using them with? Do you have other modified files (e.g., pins) that cover these issues?

#error "SERVO0_PIN must be defined for your servo or BLTOUCH probe."
#error "TMC2208 or TMC2209 on E0 requires E0_HARDWARE_SERIAL or E0_SERIAL_(RX|TX)_PIN."
#error "CUSTOM_MENU_MAIN requires an LCD controller that implements the menu."
#error "DGUS_LCD_UI IA_CREALITY requires CLASSIC_JERK."

@thinkyhead
Copy link
Member

@rmbq — Note that MachineCR10Smart is no longer used in mainline Marlin. It's been replaced with LCD_SCREEN_ROTATE 90.

@thinkyhead
Copy link
Member

@InsanityAutomation — ICYMI I also made this change (since (uint8_t)n > 0xFFFF can never be true) so you should verify on your end that it is correct:

  void RTSSHOW::RTS_SndData(const int n, const uint32_t addr, const uint8_t cmd/*=VarAddr_W*/) {
    if (cmd == VarAddr_W) {
-     if ((uint8_t)n > 0xFFFF) {
-       snddat.data[0] = n >> 16;
-       snddat.data[1] = n & 0xFFFF;
-       snddat.len     = 7;
-     }
-     else {
        snddat.data[0] = n;
        snddat.len     = 5;
-     }

@thinkyhead
Copy link
Member

See PR #25440 for the patch, where using native types for RTS_SndData resolves the ambiguity.

@rmbq
Copy link
Contributor Author

rmbq commented Feb 27, 2023

@thinkyhead yes sorry, i modified my board to support other features and i forgot to add stock configurations files. But they weren't the cause of the issue. Thanks for the patch!

@github-actions
Copy link

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.

@github-actions github-actions bot locked and limited conversation to collaborators Apr 28, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants