cskozlowski / mmu2

This arduino codebase is for an mmu2 home brew clone
GNU General Public License v3.0
73 stars 17 forks source link

gcc warning about integer overflow #4

Open KarlZeilhofer opened 5 years ago

KarlZeilhofer commented 5 years ago

The line steps = 17 * 200 * STEPSIZE; in void loadFilament(int direction) results in about 54k, which gives an signed integer overflow.

Adding ul specifiers (for unsigned long) fixes this issue: steps = 17ul * STEPSPERREVOLUTION * STEPSIZE;

cskozlowski commented 5 years ago

This was how I originally moved the filament past the PINDA sensor to get to the bondtech gear.

I no longer use this method since I have the 2nd sensor - you will see in the code that I move the filament 350 mm (way before 2nd sensor) and then go into a 'sensor loop' to wait for the filament to show up and then move it an addition 31mm (if I remember correctly) so it gets right to the middle of the bondtech gear. I now make all filament moves in mm moves (144*# of mm).

KarlZeilhofer commented 5 years ago

same for feedFilament(STEPSPERMM * 350); // go 350 mm then look for the 2nd filament sensor in void filamentLoadToMK3()

fixed it by defining STEPSPERMM as unsigned long

cskozlowski commented 5 years ago

The executable line : steps = 17 200 STEPSIZE (part of loadfilament() routine) is not called operationally and it is under 65,525 (max value for an unsigned int in the arduino).

The feedFilament call (STEPSPERMM *350) is below 65,535 so and unsigned int should be sufficient since I only need 16 bits for that value.

cskozlowski commented 5 years ago

I can say that this call - the filamentLoadtoMK3() works since it a core part of my operational code and I have had no issues with unsigned int. Changing it to unsigned long is not bad though because then I can make the bowden tube (between the MMU2 and the MK3) longer and not worry if I go past 65,535.

cskozlowski commented 5 years ago

Since you are using the leonardo board and the existing trinamic drivers. Be aware that I am using steps sizes on my 8255 set to 16 (you can see that at the top of my code). I do not know the step setup of the current trinamic drivers (it is probably either 16 or 32 so they run quieter).

KarlZeilhofer commented 5 years ago

thanks for that hint, I have to rewrite the drivers setup anyway. I will stick to your config where possible.

Prusa has configured them to halfstep (selector and pulley) and 16th step (idler)

cskozlowski commented 5 years ago

Thats good to know the production stepper rates, if I move to halfstep then I can really speed up both the extruder and selector stepper motors on my setup. I played with the stepper motor speeds but dialed them back so that it worked 100% of the time. My delay values for the stepper motors are at the top of the code and I messed with them quite a bit to get them dialed in correctly.

cskozlowski commented 5 years ago

I am excited about your port of this code base to the leonardo board. Are you going to tack on a sensor to make full use of my code or do something different?

cskozlowski commented 5 years ago

The stepper motor driver rewrite shouldn't be too difficult. I now have some trinamic stepper motor controllers in my possession so I need to try and get those up and running as well (too many projects).

KarlZeilhofer commented 5 years ago

Of course I want to use your mod with the micro switch above the extruder. That's the whole idea - and to show, that Prusa can keep their code for their own. Nobody needs such a piece of crap combined with their pride. They haven't even answered my quotation eMail within 3 days.

What is missing in your motion controll (same was the case at Prusa's code) are acceleration phases. I wrote some nice code, which does that. That's why the selector in my video can go that fast.

KarlZeilhofer commented 5 years ago

May I ask what your time zone is?

cskozlowski commented 5 years ago

GMT-5 (East Coast of US)

cskozlowski commented 5 years ago

The acceleration code was one of my 'TODO' items, I came up with a nice algorithm just hadn't implemented it yet. It really is needed for the filament feed. I think I have seen your video (youtube) showing the sped up movement of the selector and idler assembly.

What was your quotation Email to Prusa all about?

KarlZeilhofer commented 5 years ago

Sorry that I underestimated your coding skills ;) Yes, I use Qt Creator for C/C++ coding - that IDE is really very helpful! Since Arduinos integration into Platformio, and Platformio's support of Qt Creator, Arduino projects can be done in Qt Creator without losing compatibility to the Arduino IDE or having too much troubles on setting the toolchain upp.

KarlZeilhofer commented 5 years ago

Perhaps we should move our chat to a non public channel :) Any suggestions?

cskozlowski commented 5 years ago

Messenger or email ?

KarlZeilhofer commented 5 years ago

have you sent an eMail?

cskozlowski commented 5 years ago

yep ... checking the address

jettoblack commented 5 years ago

I don't have my hardware yet but I am following this FW development with much interest! I'm still thinking of making a selector-less version that uses a 5 filament combiner instead.

bula87 commented 5 years ago

Also joining the party. Today I got my Ramps board. Now I just need to print everything and assembly it :D