prusa3d / Prusa-Firmware

Firmware for Original Prusa i3 3D printer by PrusaResearch
GNU General Public License v3.0
2.02k stars 1.05k forks source link

Issue: Compiling 3.1.0 RC1 results in errors #224

Closed madhunm closed 7 years ago

madhunm commented 7 years ago

Compiling the firmware as is results in this error:

Marlin_main.cpp:5967: error: ISO C++ forbids declaration of 'update_currents' with no type [-fpermissive] update_currents() { ^ exit status 1 ISO C++ forbids declaration of 'update_currents' with no type [-fpermissive]

Declaring this update_currents() function as void results in:

In function 'void lcd_calibration_menu()': ultralcd.cpp:258: error: invalid conversion from 'bool ()()' to 'menuFunc_t {aka void ()()}' [-fpermissive] menuaction ## type ( args ); \ ^ ultralcd.cpp:3478:5: note: in expansion of macro 'MENU_ITEM' MENU_ITEM(function, MSG_SELFTEST, lcd_selftest); ^ ultralcd.cpp:192: error: initializing argument 1 of 'void menu_action_function(menuFunc_t)' [-fpermissive] static void menu_action_function(menuFunc_t data); ^ ultralcd.cpp: In function 'bool check_file(const char)': ultralcd.cpp:5781: error: invalid conversion from 'const char' to 'char' [-fpermissive] card.openFile(filename, true); ^ In file included from ultralcd.cpp:6:0: cardreader.h:23: error: initializing argument 1 of 'void CardReader::openFile(char, bool, bool)' [-fpermissive] void openFile(char name,bool read,bool replace_current=true); ^ exit status 1 invalid conversion from 'bool ()()' to 'menuFunc_t {aka void (*)()}' [-fpermissive]

madhunm commented 7 years ago

For reference, I am using Arduino IDE 1.6.8

thess commented 7 years ago

PR #138 fixes this and a whole lot of other build issues... I will keep this PR up-to-date for now. It is current and OK to apply to the latest HEAD/MK2 (3.1.0-RC1)

dot-bob commented 7 years ago

Try the latest Arduino IDE. It appears the code has been updated so it will successfully compile on the latest release.

PavelSindler commented 7 years ago

Thank you @thess for your PR and for keeping it up-to-date. I am still trying to find some time to check all those changes you made. I hope I will do it next week and merge your PR finaly. I compiled it using Arduino IDE 1.8.3.

3d-gussner commented 7 years ago

I use Arduino IDE 1.8.4 (added 'https://raw.githubusercontent.com/ultimachine/ArduinoAddons/master/package_ultimachine_index.json' in Preferences->Additional Boards Manager URLs) and get some warnings but it compiles the Prusa source code.

Here the sum of warnings i get with compiler warnings set to 'All' redefined: 264 unused variable: 76 defined but not used: 54 declared 'static' but never defined: 149 defined but not used: 54 the address of '...' will always evaluate as '...': 3 comparison between signed and unsigned integer expressions: 11 ordered comparison of pointer with integer zero : 4 unused parameter: 18 set but not used: 8 sequence out of range: 1 suggest parentheses around comparison in operand of: 30

3d-gussner commented 7 years ago

@PavelSindler It is Sunday and you are still working?

3d-gussner commented 7 years ago

@thess Just downloaded your https://github.com/thess/Prusa-Firmware/tree/warnings-removal and most of the warnings are gone!!! Thanks! Compiled with Arduino IDE 1.8.5 including the package_ultimachine_index.jason. But there are still some when setting display warnings to All (no warnings with Default or More':

Line 50: 'Path'\sketch\LiquidCrystal.cpp:85:35: warning: unused parameter 'cols' [-Wunused-parameter] Line 56: 'Path'\sketch\LiquidCrystal.cpp:163:43: warning: unused parameter 'cols' [-Wunused-parameter] Line 64: 'Path'\sketch\Marlin_main.cpp:932:37: warning: unused parameter 'quiet' [-Wunused-parameter] Line 74: 'Path'\sketch\Marlin.h:149:25: warning: suggest braces around empty body in an 'if' statement [-Wempty-body] Line 93: 'Path'\sketch\cardreader.cpp:62:45: warning: unused parameter 'prepend' [-Wunused-parameter] Line 99: 'Path'\sketch\cardreader.cpp:62:88: warning: unused parameter 'match' [-Wunused-parameter] Line 107: 'Path'\sketch\cardreader.cpp:359:25: warning: ordered comparison of pointer with integer zero [-Wextra] Line 113: 'Path'\sketch\cardreader.cpp:364:22: warning: ordered comparison of pointer with integer zero [-Wextra] Line 121: 'Path'\sketch\cardreader.cpp:457:25: warning: ordered comparison of pointer with integer zero [-Wextra] Line 127: 'Path'\sketch\cardreader.cpp:462:22: warning: ordered comparison of pointer with integer zero [-Wextra] Line 136: 'Path'\sketch\mesh_bed_calibration.cpp:2329:39: warning: unused parameter 'verbosity_level' [-Wunused-parameter] Line 143: 'Path'\sketch\mesh_bed_leveling.cpp:24:78: warning: unused parameter 'use_default' [-Wunused-parameter] Line 155: 'Path'\sketch\Marlin.h:149:25: warning: suggest braces around empty body in an 'if' statement [-Wempty-body] Line 169: 'Path'\sketch\planner.cpp:852:59: warning: suggest braces around empty body in an 'if' statement [-Wempty-body] Line 175: 'Path'\sketch\planner.cpp:853:59: warning: suggest braces around empty body in an 'if' statement [-Wempty-body] Line 181: 'Path'\sketch\planner.cpp:860:59: warning: suggest braces around empty body in an 'if' statement [-Wempty-body] Line 187: 'Path'\sketch\planner.cpp:867:59: warning: suggest braces around empty body in an 'if' statement [-Wempty-body] Line 195: 'Path'\sketch\stepper.cpp:1139:26: warning: unused parameter 'address' [-Wunused-parameter] Line 201: 'Path'\sketch\stepper.cpp:1139:39: warning: unused parameter 'value' [-Wunused-parameter] Line 209: 'Path'\sketch\ultralcd.cpp:85:31: warning: missing initializer for member 'MenuData::BabyStep::babystepMem' [-Wmissing-field-initializers] Line 215: 'Path'\sketch\ultralcd.cpp:85:31: warning: missing initializer for member 'MenuData::BabyStep::babystepMemMM' [-Wmissing-field-initializers] Line 221: 'Path'\sketch\ultralcd_implementation_hitachi_HD44780.h:1195:100: warning: parameter 'filename' set but not used [-Wunused-but-set-parameter] Line 229: 'Path'\sketch\ultralcd_implementation_hitachi_HD44780.h:1195:82: warning: unused parameter 'pstr' [-Wunused-parameter] Line 235: 'Path'\sketch\ultralcd_implementation_hitachi_HD44780.h:1254:73: warning: unused parameter 'pstr' [-Wunused-parameter] Line 241: 'Path'\sketch\ultralcd_implementation_hitachi_HD44780.h:1274:87: warning: unused parameter 'pstr' [-Wunused-parameter] Line 247: 'Path'\sketch\ultralcd_implementation_hitachi_HD44780.h:1295:78: warning: unused parameter 'pstr' [-Wunused-parameter] Line 253: 'Path'\sketch\ultralcd.cpp:5779:60: warning: unused parameter 'longFilename' [-Wunused-parameter] Line 259: 'Path'\sketch\ultralcd.cpp:5798:65: warning: unused parameter 'longFilename' [-Wunused-parameter] Line 265: 'Path'\sketch\ultralcd.cpp:6295:20: warning: unused parameter 'duration' [-Wunused-parameter] Line 271: 'Path'\sketch\ultralcd.cpp:6295:39: warning: unused parameter 'freq' [-Wunused-parameter] Line 337: 'Path'\sketch\ultralcd.cpp:5652:41: warning: '_err' may be used uninitialized in this function [-Wmaybe-uninitialized]

macfly1202 commented 7 years ago

Thank you Thess ! it's absolutely PERFECT on first look except a diff in zmax = zmin ??? in mesh_bed_calibration.cpp

Is it normal ? here my build with Arduino 1.8.5 : https://travis-ci.org/macfly1202/Prusa-Firmware

20171007

[EDIT]: some miss configuration in the travis use file : the firmware is only for mk2 MM now

thess commented 7 years ago

@macfly1202 - Good catch. Clearly the old code made no sense as zmax was uninitialized. The correct code should init zmax to something arbitrarily large - like FLT_MAX (1E+37). However, upon further examination of the code (and if I believe the comments) - This routine doesn't really work. In fact, both zmin and zmax will be identical - probably not what was intended.

This is yet another case of a block of code which in reality doesn't do anything (See: #134) and I'm not sure what happens if I fix it. Removing it will have no apparent side-effects but fixing it may.

I will fix this one in next update. EDIT: Actually, the only change needed here is to use the max function in the zmax test and the result will be correct. The code could be more efficient, but I'm trying to keep my changes to a minimum rather than introduce new glitches.

macfly1202 commented 7 years ago

Thanks Thess for comments - I try to merge your pull request https://github.com/prusa3d/Prusa-Firmware/pull/220/commits/7256ffb4b8739fb07d35f1e457701ffe040949c2 but without success due to the fact that 'removal warning was a later commit'. Just make a try to see if OK and it wasn't : haven't not much time today... Will retry tomorrow...

thess commented 7 years ago

I know there are conflicts between the warnings removal and the MBC_8POINT patches. Both PRs will apply cleanly to the main (MK2) branch separately so as to avoid a mutual dependency. If either PR is merged, then I will update the other - In the meantime, you can try the private-build branch in my repo.

thess commented 7 years ago

@3d-gussner - I usually check warnings with -Wall and not -Wextra. That said, I believe some of the warnings you have listed are worth checking into. By including additional options such as: -Wno-unused-parameter and -Wno-empty-body there are a few that could be addressed - especially the pointer signed comparisons to 0 (addresses could be negative in this respect). The others are probably not worth worrying about without changing API definitions and then the callers and then cleaning up more unused vars - another time for that.

From the GCC documentation of -Wextra:

Note that some warning flags are not implied by -Wall. Some of them warn about constructions that users generally do not consider questionable, but which occasionally you might wish to check for; others warn about constructions that are necessary or hard to avoid in some cases, and there is no simple way to modify the code to suppress the warning. Some of them are enabled by -Wextra but many of them must be enabled individually.

Thanks for feedback and testing this PR.

3d-gussner commented 7 years ago

@thess Thanks for the update! Just a question why did you deleted following things?

thess commented 7 years ago

@3d-gussner - Removal of variables set but not used/referenced. In the first instance it looks like the remnant of a cut-n-paste and then not used. The second is merely unused variables.

macfly1202 commented 7 years ago

@thess : I try your 'private-repo', latest commit and flash firmware failed (verification failed : mismatch at byte 0x3c000 0x00 != 0x81) . I'm using Travis with Arduino 1.8.5 to build it. Which version of Arduino are you using, please?

thess commented 7 years ago

@macfly1202 - Let's not hijack this thread. Please post any issues you may have with my private branches in my repo - thanks. But for now...

I generally use 1.6.13 via platformio and sometimes the arduino command-line. I rarely use the IDE unless I am setting up a new Arduino version just to verify the builds and config. I do check-build the firmware with 1.8,4 but I haven't flashed a 1.8 build - I'll try it in a bit. [Edit 9-Oct: No problem building and flashing 1.8.4 - albeit a larger image getting close to max flash size]

For flashing -- I usually use avrdude both directly and via octoprint. Are you using the IDE or the Prusa flasher? Also note, any comments about flashing the image with the bootloader are (I believe) incorrect - or at least obsolete.

macfly1202 commented 7 years ago

@Thess : ok . You true: not the good repo to post this. Sorry! I build with Travis (@probonopd Travis config file) - no IDE - using Arduino 1.8.5 version in version setup (try 1.83 too) cf https://travis-ci.org/macfly1202/Prusa-Firmware/builds/285102965 as an example. Url hex variants at the end of log file. Flashing with Prusa flasher. No issue with 3.1.0 RC1.

PavelSindler commented 7 years ago

This is issue has been fixed by https://github.com/prusa3d/Prusa-Firmware/pull/138.