MarlinFirmware / Marlin

Marlin is an optimized firmware for RepRap 3D printers based on the Arduino platform. Many commercial 3D printers come with Marlin installed. Check with your vendor if you need source code for your specific machine.
https://marlinfw.org
GNU General Public License v3.0
16.34k stars 19.26k forks source link

LCD and Babystepping results in compiling error #1221

Closed JamesT42 closed 9 years ago

JamesT42 commented 9 years ago

Downloaded bug-fixing branch on 21.12.2014 Set the following to reprodice the error:

define MOTHERBOARD BOARD_RAMPS_13_EFB

define RA_CONTROL_PANEL

define BABYSTEPPING

Get the error: ultralcd.cpp: In function 'void _lcd_babystep(int, const char*)': ultralcd.cpp:371: error: initializer fails to determine size of '__c'

It works if babystepping is not defined.

boelle commented 9 years ago

have marked this a verified bug as you use the latest we are fixing bugs on

god idea on to show how you reproduce the error... the coders will for sure track this one down a lot faster

NarimaanV commented 9 years ago

Ok so I'm not anywhere near an "advanced" programmer, and there's plenty in Marlin I still don't understand completely, but I may have found a fix for this. I basically took the baby stepping code from Printrbot's Marlin (since I tested that and it actually compiled) and transplanted it into this Marlin to replace the current Babystepping code. I also made sure to incorporate the language variables so it won't only display it in English like Printrbot originally designed on their Marlin (though "Babystepping" is apparently the same in every language on Marlin but just in case). I know now I'm supposed to do a pull request but my issue with that is that I unfortunately don't have a 3D printer yet (haven't been able to get the money for one) so there's absolutely no way for me to test the code, and I don't want to make a request to incorporate untested code as it might just completely not work for all I know. So, before I fill out a pull request, if any of the testers (or anyone in general) who have an LCD would like to try my fix out and confirm if it works, please post here and I'll create a link to a repository of my own Marlin with my fix in it.

NarimaanV commented 9 years ago

Actually funny story, turns out my "transplanting" Printrbot's babystepping code was really reverting Marlin's babystepping code back to how it was a few months back. I found an older copy of Marlin on my computer and compared it to my branch, crazy enough my changes matched up perfectly to what it used to be. Seems the main issue was someone tried to make 1 function for all the axes to use instead of 3 functions, 1 for each axis independently. Still trying to figure out exactly how that broke it since theoretically it should work fine, but in the mean time, going back to 3 functions shouldn't do any harm. I'll wait a little bit to see if anyone's eager to test, but I'll probably propose a pull request regardless since my changes don't seem so crazy anymore (given how it's how they used to be before). Worst case scenario, baby stepping doesn't work and we revert the change right?

JamesT42 commented 9 years ago

I think reverting to the old code is fine for the stable branch, and maybe someone can find the error for the new code in the development branch. because 1 routine for all axis is more logical way to go.

nophead commented 9 years ago

The error is caused by passing a variable to the PSTR macro in _lcd_babystep(). Simply remove it from the function and add it to the string constants passed to the function instead. PSTR is for making string constants in program memory. It doesn't make sense with a variable.

NarimaanV commented 9 years ago

Huh, well that explains why I couldn't find an error in the functions themselves, because they were fine to begin with. Did a little research on PSTR and that's an interesting macro. Anyways, I made the change and you're right, compiles just fine now. I'll do a pull request shortly and it'll be fixed. Thanks @nophead.

NarimaanV commented 9 years ago

And Pull Request is up. https://github.com/ErikZalm/Marlin/pull/1233

NarimaanV commented 9 years ago

The fix has been merged. We can probably close this issue, but @JamesT42 , can you try testing your configuration again now and see if Babystepping works properly?

boelle commented 9 years ago

will close this one, if @JamesT42 finds that the issue is still there then create new issue

github-actions[bot] commented 2 years ago

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.