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

Change to arduino friendly include #18880

Merged
merged 2 commits into from
Aug 4, 2020

Conversation

jeffeb3
Copy link
Contributor

@jeffeb3 jeffeb3 commented Jul 31, 2020

Requirements

  • Filling out this template is required. Pull Requests without a clear description may be closed at the maintainers' discretion.

Description

The current bugfix branch doesn't build in arduino. The issue is some new tft code uses NULL, and to get the define, they included cstddef, which isn't available with arduino. The header stddef.h is available, and also works in platformio

Benefits

The builds once again will build in arduino, not just platformio.

@rhapsodyv
Copy link
Sponsor Member

By doesn't build in arduino, do you mean Arduino IDE? Or do you mean the AVR platform (like mega2560, etc)?

@jeffeb3
Copy link
Contributor Author

jeffeb3 commented Jul 31, 2020

Arduino IDE. But also the arduino cli.

I have automated builds for the ramps, rambo and mini rambo that also include using arduino to check that build.

I have also checked with arduino 1.8.13 in linux. The error is that it can't find cstddev, and there is also an error where arduino is trying to find the library named cstddev.

From some googling, it looks like cstddev has never been available (in Arduino).

@rhapsodyv
Copy link
Sponsor Member

But this new code is supposed to work only on STM32.
I would not be compiled when using a different platform.
🤔

@jeffeb3
Copy link
Contributor Author

jeffeb3 commented Jul 31, 2020

Well. I don't know how it is included. Is there something I can provide to help figure it out?

https://github.com/jeffeb3/MarlinBuilder/actions/runs/189986919

Those are my automated builds for ramps.

I have a seperate Marlin checked out on my computer from bugfix-2.0.x, I configured it for rambo and opened the arduino IDE. It fails in the same spot with the same error. If I jist change it to stddev.h, it compiles fine.

@rhapsodyv
Copy link
Sponsor Member

If you are using platformio, those files would be removed from build.
But, as you are using arduino ide, it will try to compile.

The better fix would be: surround all this new tft code with the #define it may need enabled for work.
But is not only this file. I think it’s all this folder.

@jeffeb3
Copy link
Contributor Author

jeffeb3 commented Jul 31, 2020

Well, FWIW, this one line change has made it build for me. At least in my desktop case.

I assume the other consequence of having it build might be a larger binary, right? So that will trickle into problems for things like melzi boards.

I am sure this question comes up frequently. Is arduino still a target build environment for platforms that support it? I have another issue open related to arduino builds. I'm not actually sure if it is still desired. I'm not sure where I would look for any official announcement of something like that.

@rhapsodyv
Copy link
Sponsor Member

Binary may not get any larger because GCC is clever and discard unused symbols. But the compile time will be higher.

I don't know if your correct breaks or not the original code.

My, in my option, its better to keep the original untouched and do what I suggested.

I can create a PR with what I'm suggesting.

@jeffeb3
Copy link
Contributor Author

jeffeb3 commented Jul 31, 2020

That makes sense to me.

I think this PR also makes sense though. stddef.h is included 15 times already, and this is the first time cstddef is included.

@rhapsodyv
Copy link
Sponsor Member

Let's @thinkyhead decide

@thinkyhead
Copy link
Member

The pointy <stddef.h> include is better.

@thinkyhead thinkyhead merged commit 3a00ebd into MarlinFirmware:bugfix-2.0.x Aug 4, 2020
@Bulwark73
Copy link

I am attempting to set flash Marlin onto Mega2560 with Aduino 1.8.13 IDE. I am still getting fatal error: csddef:
Here is the whole error line;
Sketch\src\lcd\tft\tft_image.cpp:22:10: fatal error: cstddef: No such file or directory
#include "cstddef"

I downloaded the bugfix of Marlin 2.0x Yesterday morning @ 9am Chicago time.

What would I do to get Aduido IDE to stop giving me this error?

@thisiskeithb
Copy link
Member

What would I do to get Aduido IDE to stop giving me this error?

Download a fresh copy of bugfix-2.0.x or manually edit Marlin/src/lcd/tft/tft_image.cpp as shown:

- #include "cstddef"
+ #include <stddef.h>

@Bulwark73
Copy link

YES. I wondered the same when I saw when thinkyhead had updated it. It is working now. All is good in the Nado verse now. ;) I appreciate this site and the information was easy to find. I am new to the site. Just getting use to where the times are listed. I must have downloaded my version just before he updated it. Oh well, Live and learn.

Speaka pushed a commit to Speaka/Marlin that referenced this pull request Aug 13, 2020
albertogg pushed a commit to albertogg/Marlin that referenced this pull request Aug 31, 2020
vgadreau pushed a commit to vgadreau/Marlin that referenced this pull request Dec 9, 2020
ahmetcemturan pushed a commit to ahmetcemturan/Marlin that referenced this pull request Jan 21, 2021
ahmetcemturan added a commit to ahmetcemturan/Marlin that referenced this pull request Jan 23, 2021
kageurufu pushed a commit to CR30-Users/Marlin-CR30 that referenced this pull request Apr 30, 2021
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.

None yet

5 participants