Marlin firmware fork for MakerFarm i3v w/ auto bed level & Roxy's enhancements

[Roxy]

Yes, this is the biggest problem right now. It won't affect anyone in the MakerFarm family, so far as I know, but it does prevent the change from being pushed back to trunk without further consideration. I think it will also have an impact on any post-processing scripts. This really makes me want to rename or reannotate the L, F, R, B parameters to reflect that they are really Xmin, Ymin, Xmax, Ymax.

Actually, my machine was originally setup with the origin in the front left. But there were problems with the least square fit of the Auto Bed Leveling in the early versions where they could not handle negative coordinates. The easiest answer was to rotate my origin to the back right. Then my probe was always in positive space. What I'm arguing is the type of machine may not insulate you from the issue.

I think having the parameters named Xmin, Ymin, Xmax, Ymax is much clearer. The big problem is none of the other GCode commands use long parameter names. And there is no support for the longer parameter names. The support code would not be difficult to do. But....

One idea would be to change the Code_Seen(char ) function to be Code_Seen(char *). It would be very quick to go through the code base (and probably it is just Marlin_main.cpp) to put double quotes around all the existing characters in Code_Seen() functions. We could leave everything else the same where it leaves a pointer looking at the numeric value for the hunted parameter. If we did that... Xmin, Ymin, Xmax, Ymax would work nicely and avoid a lot of confusion for people.

Right now, Code_Seen() is pretty trivial:

bool code_seen(char code)
{
strchr_pointer = strchr(cmdbuffer[bufindr], code);
return (strchr_pointer != NULL); //Return True if a character was found
}

If we went that way... It would turn into something like this. But we may want to desensitize the comparison to upper and lower case. I don't know.

bool code_seen(char *code)
{
strchr_pointer = strstr(cmdbuffer[bufindr], code);
return (strchr_pointer != NULL); //Return True if the specified parameter was found
}

[printbus]

...This really makes me want to rename or reannotate the L, F, R, B parameters to reflect that they are really Xmin, Ymin, Xmax, Ymax.

The idea of Xmin, Ymin, Xmax, and Ymax seems so much clearer. EDIT: Stated before I read Roxy's post on limitations.

In tune with calling things really what they are, I'd recommend using n-by-n text in lieu of n x n in the comments. Those unfamiliar could think x is a variable, or an axis...

...So, I have setup a fork of the Marlin trunk with the Type 6 therm. table modified, auto bed leveling enabled (this is easy to turn off) and Roxy's enhanced G29 (and G28) commands with the debugging info on. For a better description of the therm table stuff, see this post.

To facilitate merging the updates back into the Marlin trunk, I'd reconsider whether the fork uses the MakerFarm changes to the Type 6 thermistor. The Marlin trunk isn't going to want the Type 6 table changed - there could be people out there used to how it measures temperatures. Your configuration.h file also still contains the now incorrect comment about Type 6 not being as accurate as Type 1 and being created with a Fluke thermocouple. My vote would be to revert thermistortables.h back to the Marlin Type 6 data and change configuration.h to use Type 1 thermistors. Beyond what I concluded in the referenced thread, I think someone said zennmasterM has also said it's OK for MakerFarm people to use Type 1 when they update their firmware as part of his video on auto leveling.

EDIT: BTW - I've been using Type 1 just fine the last week or so. If, for some reason, we're uncomfortable switching to Type 1, the MakerFarm thermistor table could at least be defined as a new thermistor type that is used rather than modifying the Marlin type 6 table.

[Roxy]

Dacb: Random new topic here: You modified the code to allocate space this way:
(I might have already added this code to your branch of the Marlin tree... But I could not Sync it)

// "A" matrix of the linear system of equations
double* eqnAMatrix = (double*)malloc(sizeof(double) * (n_points*n_points*3));
// "B" vector of Z points
double* eqnBVector = (double*)malloc(sizeof(double) * (n_points*n_points))

This really is a better way of doing it than assumming the stack is deep enough to handle those large arrays. But we probably should check that the memory was available on the heap. We might want to add right after the malloc()'s something like:

if ( eqnAMatrix==NULL || eqnBVector==NULL) {
            SERIAL_PROTOCOLPGM("?Memory not available for Auto Bed Leveling.\n");
            if (eqnAVector)
                 free(eqnAVector);
            if (eqnBVector)
                 free(eqnBVector);
            break;
}

and the memory isn't free()'ed at the end. At some point, the Arduino board won't be able to do Auto Bed Leveling any more!

#endif  // ORIGIN_FRONT_RIGHT
}
            set_bed_level_equation_lsq(plane_equation_coefficients);

            free(plane_equation_coefficients);
            free(eqnAVector);
            free(eqnBVector);
#else // AUTO_BED_LEVELING_GRID not defined

Mean while.... I've got my post processing script for Slic3r finding the print's size and modifying the G29 in the GCode file. I guess I'll use F, B, L, R for right now... But I think my vote is for using XMIN, YMIN, XMAX and YMAX.

[TopJimmyCooks]

Forgive me if this is already addressed, I did not read every post thoroughly. I really need to be able to set a small Z raise before retracting the Z probe during ABL. At each point of bed probing, the probe is lowered and then retracts before moving again. The bottom of the switch housing sometimes hits the bed and sometimes hangs up. This could be fixed differently/mechanically but it seems like something pretty easy.

I also have a problem sometimes where if I have done a print that is only 2-3 layers tall, such as a business card, and then home Z, the nozzle is too low to allow the Z probe to extend. My ABL z offset is 5.6 mm currently, so this could happen on any print that ends at less than that height. It would be wonderful if this code could address raising Z by the probe offset plus a bit to avoid this.

I expect that if Roxy's keep probe extended setting is still an option this has already been considered. Again, thanks for all the work guys, it is appreciated greatly by us non-coders.

[Roxy]

Forgive me if this is already addressed, I did not read every post thoroughly. I really need to be able to set a small Z raise before retracting the Z probe during ABL. At each point of bed probing, the probe is lowered and then retracts before moving again. The bottom of the switch housing sometimes hits the bed and sometimes hangs up. This could be fixed differently/mechanically but it seems like something pretty easy.

I also have a problem sometimes where if I have done a print that is only 2-3 layers tall, such as a business card, and then home Z, the nozzle is too low to allow the Z probe to extend. My ABL z offset is 5.6 mm currently, so this could happen on any print that ends at less than that height. It would be wonderful if this code could address raising Z by the probe offset plus a bit to avoid this.

I expect that if Roxy's keep probe extended setting is still an option this has already been considered. Again, thanks for all the work guys, it is appreciated greatly by us non-coders.

Yes, that is already handled in the Enhanced G29 code. And that can be obtained here: http://3dprintboard.com/showthread.p...ed-G29-command

But my suggestion is for you to wait a few days, because we are very close to having some other enhancements in place in the G29 and a specific version for the MakerFarm.

[TopJimmyCooks]

Again, thanks. I'll eagerly look forward to that update.

[printbus]

What? Somebody doing a malloc in Arduino code? Man, my respect for dacb just went up a few notches.

[Roxy]

Dacb: I might have successfully cloned your latest branch of the Marlin code on my machine. I made the malloc() changes. But it would not let me sync them. The sync fails because I do not have permissions. Can you grab the code I added?

Also, I attempted to push my Slic3r G29 Post Processing script up into your PlugIns directory. I don't know if that worked either. Can you see it and grab it?

[dacb]

...I think having the parameters named Xmin, Ymin, Xmax, Ymax is much clearer. The big problem is none of the other GCode commands use long parameter names. And there is no support for the longer parameter names. The support code would not be difficult to do. But....


One idea would be to change the Code_Seen(char ) function to be Code_Seen(char *). It would be very quick to go through the code base (and probably it is just Marlin_main.cpp) to put double quotes around all the existing characters in Code_Seen() functions. We could leave everything else the same where it leaves a pointer looking at the numeric value for the hunted parameter. If we did that... Xmin, Ymin, Xmax, Ymax would work nicely and avoid a lot of confusion for people...

I looked through a bunch of documents about G-code and "standards" and I don't feel that multi-letter parameters is in the spirit. I would propose I, J, K, and L. These are clearly an array but not overly misleading in specificity (e.g. L,F,R,B). Each of these followed by a value in mm.

To facilitate merging the updates back into the Marlin trunk, I'd reconsider whether the fork uses the MakerFarm changes to the Type 6 thermistor. The Marlin trunk isn't going to want the Type 6 table changed - there could be people out there used to how it measures temperatures. Your configuration.h file also still contains the now incorrect comment about Type 6 not being as accurate as Type 1 and being created with a Fluke thermocouple. My vote would be to revert thermistortables.h back to the Marlin Type 6 data and change configuration.h to use Type 1 thermistors. Beyond what I concluded in the referenced thread, I think someone said zennmasterM has also said it's OK for MakerFarm people to use Type 1 when they update their firmware as part of his video on auto leveling.


EDIT: BTW - I've been using Type 1 just fine the last week or so. If, for some reason, we're uncomfortable switching to Type 1, the MakerFarm thermistor table could at least be defined as a new thermistor type that is used rather than modifying the Marlin type 6 table.

I have to agree with this. Going back to master on thermistors just makes sense, here is the commit: https://github.com/beckdac/Marlin/co...f473eed821097b

Dacb: Random new topic here: You modified the code to allocate space this way:...

I can't actually take credit for that code, it is from https://github.com/ErikZalm/Marlin/pull/1022


Good catch on the missing free.

What? Somebody doing a malloc in Arduino code? Man, my respect for dacb just went up a few notches.

I'm a die hard 'avr-gcc/avr-libc and nice tight hand crafted libs on a Tiny' kind of guy, so I have to confess I don't use the Arduino environment for my AVR projects (but I love AVRs!). Hate the idea of loosing timers and the overhead. I guess with the 2560, this isn't really a problem.

Dacb: I might have successfully cloned your latest branch of the Marlin code on my machine. I made the malloc() changes. But it would not let me sync them. The sync fails because I do not have permissions. Can you grab the code I added?
Also, I attempted to push my Slic3r G29 Post Processing script up into your PlugIns directory. I don't know if that worked either. Can you see it and grab it?

Can you PM me your github username? I'll add you as a collaborator so your commits give you the recognition you deserve!

[Roxy]

I looked through a bunch of documents about G-code and "standards" and I don't feel that multi-letter parameters is in the spirit. I would propose I, J, K, and L. These are clearly an array but not overly misleading in specificity (e.g. L,F,R,B). Each of these followed by a value in mm.

I understand your thoughts! And I 3/4 agree. The big problem is these one letter variable names cause all kinds of problems. And given the code cost is so minimal to support long parameter names, I'm actually thinking it makes sense to make a transition. After all... If somebody doesn't want to have a longer, clearer variable name in the G29 command, they don't need to use it. But with that said, I'm perfectly fine moving to some parameter names that mean nothing and add no confusion. It will be easy to document that I is the minimum X-Axis coordinate to probe. J is the maximum X-Axis coordinate to probe, etc, etc. What we really are trying to do is set this up so people aren't pulling their hair out trying to get it working. (and longer parameter names would do that just as going with I,J,K & L will do it. )

Can you PM me your github username? I'll add you as a collaborator so your commits give you the recognition you deserve!

I don't need credit... But being able to make updates is helpful. I sent you a Private Message. But isn't all that stuff public anyways? Anybody that is using a fork in the tree can trace it back to every change that is made and who made it, right?

What are your thoughts on the Post Processors? The problem is even though I'm a big proponent of Open Source, I do use MSVC Studio for most of my C code when I'm playing around. I put the whole Slic3r Post Processor 'MSVC Solution' into your PlugIn folder. I deliberately made the thing very straight forward C code that uses nothing but generic library functions. It doesn't even have a header file. So it will compile with just about anything. But with that said... I'm wondering if the Open Source fans are going to be hostile about that? I guess if they don't like it, they can spend the time to take the single .C file and set up a GCC environment for it.