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

gcc warning about integer overflow #4

Open
KarlZeilhofer opened this issue Oct 18, 2018 · 20 comments
Open

gcc warning about integer overflow #4

KarlZeilhofer opened this issue Oct 18, 2018 · 20 comments

Comments

@KarlZeilhofer
Copy link

KarlZeilhofer commented Oct 18, 2018

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
Copy link
Owner

cskozlowski commented Oct 18, 2018

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
Copy link
Author

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
Copy link
Owner

cskozlowski commented Oct 18, 2018

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
Copy link
Owner

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
Copy link
Owner

cskozlowski commented Oct 18, 2018

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
Copy link
Author

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
Copy link
Owner

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
Copy link
Owner

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
Copy link
Owner

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
Copy link
Author

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
Copy link
Author

May I ask what your time zone is?

@cskozlowski
Copy link
Owner

GMT-5 (East Coast of US)

@cskozlowski
Copy link
Owner

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
Copy link
Author

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
Copy link
Author

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

@cskozlowski
Copy link
Owner

Messenger or email ?

@KarlZeilhofer
Copy link
Author

have you sent an eMail?

@cskozlowski
Copy link
Owner

yep ... checking the address

@jettoblack
Copy link

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
Copy link

bula87 commented Nov 8, 2018

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants