SD Card & LCD Support Broken

[bluesign2k]

Enabling either SDSUPPORT or any of the LCD panels with SD support now fails to compile. The IDE reports that LONG_FILENAME_LENGTH is not declared. It seems SdFile.h is missing. From a fresh clone, the error can be easily replicated simply by selecting a motherboard that supports LCDs (eg BOARD_RAMPS_13_EFB) and an LCD (eg. REPRAP_DISCOUNT_SMART_CONTROLLER).

[bkubicek]

I think its unacceptable that any branch has a state, where something that basic does not work.

Bernhard

On Thu, Jan 15, 2015 at 2:46 PM, Chris notifications@github.com wrote:

Enabling either SDSUPPORT or any of the LCD panels with SD support now fails to compile. The IDE reports that LONG_FILENAME_LENGTH is not declared. It seems SdFile.h is missing. From a fresh clone, the error can be easily replicated simply by selecting a motherboard that supports LCDs (eg BOARD_RAMPS_13_EFB) and an LCD (eg. REPRAP_DISCOUNT_SMART_CONTROLLER).

— Reply to this email directly or view it on GitHub https://github.com/MarlinFirmware/Marlin/issues/1366.

[monkeydave]

Not near a computer but one of the commits this morning removed the SD library from the marlin directory. You will likely need to add the library to your Arduino IDE enviroment the same as you would for the LCD libraries.

The files are in Marlin/ArduinoAddons/Arduino_1.0.x/libraries/SdFat

[bluesign2k]

Like @bkubicek I find it genuinely astounding that such a fundamental and obvious bug has slipped through the net. I have to admit that I've been skeptical of the quality of a number of the pull requests recently, but more so with the apparent speed at which these requests are getting checked a merged. Frankly, I'm glad I have a few old copies of marlin about. I digress...

@monkeydave, Marlin shouldn't require adding any extra libraries to the IDE. Anyone should be able to clone the repo, change Configuration.h for their local setup and hit compile and have it work.

[boelle]

me can agree on this one, even if i did the merge...

but we still have a problem.... to few coders that can fix errors... to many that opens issues due the first problem (more or less that is)

when i came to the project there was issues that was left alone untouched and about 2-3 years old....

of course when you stir up in all this things are going to happen... and the more active coders there are the faster new issues can be solved

[monkeydave]

I dont disagree with what you and @bkubicek are saying, just trying to help you out.

Could you try git checkout f2cb4a3655399d68bbf655aa49adf2e923d4e43e and see if it compiles?

The merge could be reverted by a collaborator, as a quick fix. What were the advantages to this change anyway?

[boelle]

the only i can see is that such a lib is not mixed within the code... and if there is an update its easy done...

it could as well have been thrown in a "standard" lib folder...

[NarimaanV]

"Marlin shouldn't require adding any extra libraries to the IDE." @bluesign2k Thing is, it already does. Now the idea of not having to add libraries to the IDE is a whole separate and bigger issue, but since people already have to to use some of LCD's, I don't see the big deal with adding one more library to the IDE, and it's something you only do once. And at the moment, I think we need to take as much as we can out of the actual code for now, so maybe it becomes easier to clean up as soon as it's stable.

[monkeydave]

Just a few notes, as thought it was PR #1357 that caused this. After looking at the code I think there are a few other problems with this PR.

The Marlin.ino hasn't been updated like Marlin.pde so could also cause problems. The makefile hasn't been updated with the new location for the library so will also fail.

I just uploaded a quick video to youtube for anyone who doesn't know how to add libraries to the Arduino IDE and needs to get around this bug https://www.youtube.com/watch?v=PLZEGI_v0hk

[odewdney]

So should the configuration.h file section for LCD/SD support have a comment informing the person enabling SD support that they will need to add the library to their config?

[bkubicek]

no, because it is not acceptable to have users fuddle around.

Bernhard

On Fri, Jan 16, 2015 at 11:01 AM, odewdney notifications@github.com wrote:

So should the configuration.h file section for LCD/SD support have a comment informing the person enabling SD support that they will need to add the library to their config?

— Reply to this email directly or view it on GitHub https://github.com/MarlinFirmware/Marlin/issues/1366#issuecomment-70232084 .

[justmyopinion]

@bkubicek , I do not understand your point here and stating a principle here would stop further progress which has no meaning. a lot of people in this community are "fuddling around" and has the ability to install libraries, particulary with different LCD's and panels so do not worry but make a proper documentation on how to do this and the problem goes away. I think that taking SDFAT library out of source was the right thing to do and the errors with this could most likely be found easily with a minor work. Keep up the good work guys. (please also kill that Hardwareserial_h name and remove the "fuddling" with similar library name)

[bluesign2k]

I think what he means is (and as I said before), user shouldn't have to mess around downloading or installing extra libraries. You you just be able to download it, change settings to suit your printer and then upload.

@NarimaanV I hadn't realised that some LCDs already needed extra libraries adding. Are they ones that are included within the "ArduinoAddons" folder or do they have to be downloaded from elsewhere? What's the motivation for do it this way anyway?

Regarding the SdFat library, is there any particular reason why Marlin doesn't use the SD library that ships with the Arduino IDE?

[nophead]

Using anything that ships with the IDE is a nightmare because the libraries are always changing and not in backwards compatible ways. In order to get a consistent build the libraries used by Marlin should be shipped with Marlin, as they are now. Otherwise everybody is running slightly different code and bug reports become a nightmare to reproduce.

On 16 January 2015 at 12:04, Chris notifications@github.com wrote:

I think what he means is (and as I said before), user shouldn't have to mess around downloading or installing extra libraries. You you just be able to download it, change settings to suit your printer and then upload.

@NarimaanV https://github.com/NarimaanV I hadn't realised that some LCDs already needed extra libraries adding. Are they ones that are included within the "ArduinoAddons" folder or do they have to be downloaded from elsewhere? What's the motivation for do it this way anyway?

Regarding the SdFat library, is there any particular reason why Marlin doesn't use the SD library that ships with the Arduino IDE?

Reply to this email directly or view it on GitHub https://github.com/MarlinFirmware/Marlin/issues/1366#issuecomment-70244791 .

[galexander1]

Just to give a concrete example for @nophead's point, there is the disabling of interrupts when writing EEPROM -- which some (most?) avr libraries do, but maybe didn't in older versions, and became a still-not-really-nailed-down Marlin bug report.

Not advocating to change how we write to EEPROM because of it, but if we switch to an external sd library then we will face that same sort of problem in this area as well.

[justmyopinion]

@nophead , I understand what you are saying, but reality is against you. if you are using Arduino IDE take a look of the huge filelists which are unscrollable and hard to get into or overview. Most files are never touched. Recent "progress" was adding several language files to make it even worse. not a progress IMO. I would never have to use "russianCrystals" or any "Blink" device or whatever obsolete files present, but I have to live with it. If libraries were hidden linkable files it would be perfect, like objects which you could built upon but not change it would suit me a lot better, but that is not at the moment reality. If functionality like SD lib could be used from IDE installation, it would not be a huge offer to claim version dependicy (which is already present), at least my opinion. i could wish that the setup utility that Repetier has established online could be implemented for marlin as well, which could take some pain out of installations.

[galexander1]

@justmyopinion I hope this is not controvertial, but I think as a rule of thumb we do not make substantive changes to the code base just because the Arduino IDE is awful. We may make minor changes (renaming files), and of course we have to fix any actual bugs that are caused by Marlin's interaction with the IDE. But "Arduino IDE is fundamentally against the user" is not a bug in Marlin.

[nophead]

@justmyopinion, there is a difference between coping the library and hacking it in the IDE, as currently the SDFAT library, and copying it verbatim into the libraries folder under ArduinoAddons as is currently done with the LiquidCrystal library.

The former clutters the crappy IDE but is necessary if it needs tweaking to work with Marlin. The latter keeps all the advantages of it being a separate library but ensures the correct version is used. It does require the user to copy it to the right place but that is already the case for a lot of platforms and a lot of displays.

[bluesign2k]

@nophead 's comment here: "In order to get a consistent build the libraries used by Marlin should be shipped with Marlin, as they are now. Otherwise everybody is running slightly different code and bug reports become a nightmare to reproduce." is exactly my thinking.

I think the argument for stripping out files just to make the code easier to work with on a crap IDE is ridiculous. I can't believe that anyone actually uses the Arduino IDE to do any editing. Surely the norm is to use something like Notepad++ or Sublime text (or even VS with the Visual Micro plugin) to do the editing and then only load the IDE to compile an program. If that's not the case then I can see why people are so desperate to reduce the number of files!

[bkubicek]

@cris: >>Regarding the SdFat library, is there any particular reason why Marlin doesn't use the SD library that ships with the Arduino IDE?<<

Yes there was a reason why I could not use it, and it was a very good reason. Sadly, I really can't remember, its quite a while ago. That was before somebody added the long filename stuff, so also that is not the reason.

Bernhard

On Fri, Jan 16, 2015 at 2:29 PM, Chris notifications@github.com wrote:

@nophead https://github.com/nophead 's comment here: "In order to get a consistent build the libraries used by Marlin should be shipped with Marlin, as they are now. Otherwise everybody is running slightly different code and bug reports become a nightmare to reproduce." is exactly my thinking.

I think the argument for stripping out files just to make the code easier to work with on a crap IDE is ridiculous. I can't believe that anyone actually uses the Arduino IDE to do any editing. Surely the norm is to use something like Notepad++ or Sublime text (or even VS with the Visual Micro plugin) to do the editing and then only load the IDE to compile an program. If that's not the case then I can see why people are so desperate to reduce the number of files!

— Reply to this email directly or view it on GitHub https://github.com/MarlinFirmware/Marlin/issues/1366#issuecomment-70253148 .

[nophead]

@bluesign2k, I think most users only need to edit the config so do use the IDE. Developers probably use something better but the ratio of users to developers tends towards infinity.

[bkubicek]

I am not absolutely opposed to move parts of marlin into libraries, I did actually proposed it quite often for more advanced firmwares. It is a very important step, so code can be shared between different ecosystems. What I expect however: a) It should compile automatically, and be selfcontained. All libs should be in the build folders, and be used without interfering with other build setups. Not only from a user perspective, but also from a programmer perspective. b) any change in a library should be upstreamable to all software tools that use this library. So, it should for that be a dedicated git. However, this to my knowledge collides with a) Userfriendlyness, and tracability of bugs here mandates for a) c) A library should be as hardware independent as possible (read: ARM compatible), or at least have a call-compatible version on each platform. d) it should compile using arduino. We could get rid of quite some problems by having makefiles, and using avr-gcc directly. However, its not as user friendly. There is very little dependency on arduino anyhow.

I know of no way of having these combined. Sadly.

My design goal with the headers and start of the c++ files was to have a clear boundary between individual files, where you need to declare whats global and whats local. This was a milestone on the way to real libraries. Evdz and me discussed a lot if other parts of the code should be more encapsulated, e.g. class-oriented. That however was after I spend half a year trying to do the lcd-menu system in OOP way, and only managing to get it to work by skipping as much dynamic memory management, as possible, and by that also stepping back from the class-oriented design. After all, the microcontrollers are limited, sadly.

Bernhard

On Fri, Jan 16, 2015 at 2:33 PM, Bernhard Kubicek < bernhard.kubicek@gmail.com> wrote:

@cris: >>Regarding the SdFat library, is there any particular reason why Marlin doesn't use the SD library that ships with the Arduino IDE?<<

Yes there was a reason why I could not use it, and it was a very good reason. Sadly, I really can't remember, its quite a while ago. That was before somebody added the long filename stuff, so also that is not the reason.

Bernhard

On Fri, Jan 16, 2015 at 2:29 PM, Chris notifications@github.com wrote:

@nophead https://github.com/nophead 's comment here: "In order to get a consistent build the libraries used by Marlin should be shipped with Marlin, as they are now. Otherwise everybody is running slightly different code and bug reports become a nightmare to reproduce." is exactly my thinking.

I think the argument for stripping out files just to make the code easier to work with on a crap IDE is ridiculous. I can't believe that anyone actually uses the Arduino IDE to do any editing. Surely the norm is to use something like Notepad++ or Sublime text (or even VS with the Visual Micro plugin) to do the editing and then only load the IDE to compile an program. If that's not the case then I can see why people are so desperate to reduce the number of files!

— Reply to this email directly or view it on GitHub https://github.com/MarlinFirmware/Marlin/issues/1366#issuecomment-70253148 .

[justmyopinion]

@bkubicek I agree with most of your statements but confronted with reality always leave room for compromises caused by missing resources or other limitations. As i pointed out implementation here of LCD support is filled with these limitations as users claim new targets as time goes by. I have to add TWI2 library to get my Vicki display functional other users have to do different. Should this code also be part of Marlin codebase? i agrre that a class oriented approach would be best , but that involves probably a lot of compromises already also due to resources and hw related stuff.

[NarimaanV]

@bluesign2k Lol, I'll admit, forgot those were in that folder too, I just downloaded them using the links provided within Configuration.h. But yeah, those are the ones I was referring to.

Also, crazy idea here, and feel free to shoot me down on this since its probably a lot harder said than done and will probably cause more problems than solutions, but what if, similar to what PxT did with the preconfigured Arduino IDE on Mac for the Printrboard, we create a preconfigured Arduino IDE for Marlin that include all the libraries we would possibly need? Then, when someone comes to download Marlin, there can be a link to that IDE and a note saying "Download this IDE and open Marlin using it". Again, just a thought that came to my mind, I'm sure someone'll point out a pretty good reason that's not an ideal solution.

[odewdney]

May we should just wait until this Merge request is accepted into the Arduino IDE: https://github.com/arduino/Arduino/pull/1986 That will include libraries that are sub-folders of the sketchbook, and will allow the code to 'just work'

As this is the 'Development' branch there should be some scope for 'fuddling'

[boelle]

regarding dev branch... people should really understand that we are in a process of fixing bugs that have been arround for years...

and by patience i mean they should understand that this is not a paid for product... and any dev work takes time... and there are so far only a limited number of people that can and are willing to work on the project for free...

but people are better at barking at us than helping out... i see more issues raised than pull requests that fixes the problems in the milestones which i still think are the most serious ones

[lordofhyphens]

@NarimaanV One of the reasons to not package our own variant of the Arduino IDE is that it seems very inefficient to me--I think that having simple instructions so that the user can set up their own environment correctly is better than trying to do everything for them. We don't have the people available now to keep up with the requests, maintaining our own semi-custom Arduino environment wouldn't help.

[NarimaanV]

@lordofhyphens I completely agree with you. My idea was in response to the idea of the user not having to install libraries on their own, so instead of including them all in Marlin, we'd include them in the IDE. I honestly think asking the users to install the libraries in their own IDE's isn't too much to ask at all, and it isn't very complicated either (Select "Add Library" and select the library's folder or zip file, done), especially considering 3D printing still is a "tinkerer's hobby" and requires some basic knowledge of the Arduino IDE and the software/firmware that drive 3D printers.

[lordofhyphens]

@NarimaanV Additionally, for 80-90% of the users, their Marlin configurations are done exactly once.

For anything that needs a lib, the instructions should be with the configuration option in Configuration.h

[NarimaanV]

^^^ +1 Definitely a good idea.

[thinkyhead]

Since I hope to make SDFat (beta) an external library dependency soon, I'm certainly behind this concept. Marlin already needs u8g and SPI externals. It's just that the SDFat sources included with Marlin have been customized. But the features that were patched in (esp. long filename support) seem to be implemented in the beta, which will eventually (soon?) be the release version.

[daid]

@thinkyhead NO NO NO. DO NOT MAKE THE FAT/SD LIBRARY AN EXTERNAL DEPENDENCY. If you do this, I will pull my hands out of the project, and you will piss off a lot of other people as well. It's core functionality and it has work out of the box. ErikZalm is with me on this one. And I think most other maintainers will be as well.

[Grogyan]

I agree, do not make it an external dependency. As SD support is now such an integral part of 3D printing, that it will only hurt the community. It does need to be clearer in how does integrate with the firmware.

[ErikZalm]

@thinkyhead Do not make the SD/Fat library external. It is a big risk for no functional gain.

[thinkyhead]

Putting it out there for your consideration is just a part of this joyous process. Thanks for the feedback!

The gist of my interest in SDFat is to take advantage of any beneficial updates it has received since it was first integrated with Marlin. So, perhaps the SDFat code included in the Marlin package can be updated with improvements from the SDFat project. I am in the process of studying the old and new SDFat code to see what's what.

[galexander1]

Curious - what are the beneficial updates?

[thinkyhead]

Long filename support is now properly integrated, for one thing. Not too important for us, but ARM support was added also. Various other changes which (I can only presume at this point) are based on the developer having some more experience in this milieu.

My aim is to try using the new version of the library and see what the result looks like. If it looks better, then it will be kept. If it doesn't provide any especial improvement, it will be set aside.

[github-actions[bot]]

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.