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.35k stars 19.26k forks source link

Arduino Experts: Question about breaking Marlin up into modules #3868

Closed boelle closed 8 years ago

boelle commented 8 years ago

Issue by Roxy-3DPrintBoard Thursday Mar 17, 2016 at 22:41 GMT Originally opened as https://github.com/MarlinFirmware/MarlinDev/issues/395


This is a question to the Arduino Experts:

Does the ability to have a sketch be comprised of separate folders allow us to break up the code into separate modules. In the past, we had to have everything in the same directory, and in fact, currently some of the files (like Marlin_main.cpp) have kept getting bigger and bigger because we can't seem to break them up.

We started down a path of turning Marlin into a library just so we could start breaking it up into modules. But there are a lot of limitations to going that path. First, you can only place the sketch in C:\Users\name\Documents\Arduino and even worse, it won't work if there are other sketches there.

The release notes seem very incomplete and I can't find information within the Arduino project on how to even use the multiple folders that it supposedly supports.

Can somebody jump in and enlighten us?

boelle commented 8 years ago

Comment by MagoKimbra Friday Mar 18, 2016 at 07:34 GMT


If return everything in a normal sketch, now with Arduino IDE 1.6.7 you can include files in folders and sub-folders ... Example:

include "module/motion/stepper.h"

Include "module/language/english/language.h"

boelle commented 8 years ago

Comment by Roxy-3DPrintBoard Friday Mar 18, 2016 at 13:13 GMT


I don't think we have a need to do it... Can we use paths that are up above and outside of the folder that the sketch is in?

And perhaps a more important question: With this new capability, we don't have any reason to generate a library file any more in order to enable us to break up Marlin into small pieces, right?

boelle commented 8 years ago

Comment by MagoKimbra Friday Mar 18, 2016 at 20:23 GMT


Exactly ... If you look at my fork is done right so ... https://github.com/MagoKimbra/MarlinKimbra

boelle commented 8 years ago

Comment by thinkyhead Friday Mar 18, 2016 at 21:04 GMT


I want very much to organize in subfolders, and the MagoKimbra layout is a good model, but I felt that Marlin 1.1.0 should be compatible with older versions of Arduino and other IDE systems that don't have the 1.6.7 toolset yet. Basically, because the community deserves a really good release following the old paradigm before we force them to move to a new one.

Would it make sense to (1) call Marlin with the new layout 1.1.1, or 1.2.0, and (2) should we curate a version of Marlin that uses the older layout for the convenience of users who prefer or are stuck with older toolsets?

boelle commented 8 years ago

Comment by Roxy-3DPrintBoard Friday Mar 18, 2016 at 21:24 GMT


I want very much to organize in subfolders, and the MagoKimbra layout is a good model, but I felt that Marlin 1.1.0 should be compatible with older versions of Arduino and other IDE systems that don't have the 1.6.7 toolset yet. Basically, because the community deserves a really good release following the old paradigm before we force them to move to a new one.

@thinkyhead When we get done with the Release Candidates, we should take a look at how hard it would be to merge all of the bug fixes in it to the file structure that MagoKimbra has. It is probably too much to hope for, but wouldn't it be nice to have the best of both worlds???

boelle commented 8 years ago

Comment by jbrazio Friday Mar 18, 2016 at 21:34 GMT


As Marlin is dependent on Arduino as it's toolkit, if the toolkit evolves I do not believe the burden of curating legacy Marlin code should fall into core team hands. But I do agree with @thinkyhead where a transition period should be somehow in place in order not to unleash user rage. A stable release shall be then marked as the final one supporting Arduino toolkit 1.x and the team must move forward following mainstream.

boelle commented 8 years ago

Comment by Roxy-3DPrintBoard Friday Mar 18, 2016 at 21:44 GMT


As Marlin is dependent on Arduino as it's toolkit, if the toolkit evolves I do not believe the burden of curating legacy Marlin code should fall into core team hands. But I do agree with @thinkyhead where a transition period should be somehow in place in order not to unleash user rage. A stable release shall be then marked as the final one supporting Arduino toolkit 1.x and the team must move forward following mainstream.

I'm not sure I understand what you are trying to say. But here is what I'm thinking...

If we can start moving functionality into separate folders it makes it much easier to start cleaning up the code. We would still be using Arduino, and to the user the only thing that changed is the organization of the files when they unzip the .ZIP file to build the sketch.

In fact, I think we would still try to have a configuration.h file that controlled the bulk or high level of the configuration. Where we might want to have discussions is on topics like Auto Bed Leveling where there are so many sub-features that need to be tuned. It may make sense to have reasonable defaults but bury those down in the folder where they get used. And if you need to tweak a setting from its (reasonable) default, you would not find that in the main configuration.h file. But like I say, that is just my thinking of how we might want to organize things assuming we can start breaking up the code into modules.

If we went this route, that is probably very easy for the user. When they upgrade to a version of the firmware that is organized around the new file layout, they would do what they always do. They would do a diff on their configuration.h file and compare it to the new configuration.h file to see if there are any new settings they need to cross over. And then they compile and load it...

I don't think trying to cross our RCBugFix's over to the file layout that MagoKimbra has will cause issues for the users.

boelle commented 8 years ago

Comment by jbrazio Friday Mar 18, 2016 at 21:58 GMT


Maybe I got it wrong, but if we change the layout then the src code is incompatible with previous versions of Arduino right ? This was the context of my answer but once again I may have got the wrong understanding.

boelle commented 8 years ago

Comment by jbrazio Friday Mar 18, 2016 at 21:59 GMT


Overall I agree with your approach Roxy.

boelle commented 8 years ago

Comment by Roxy-3DPrintBoard Friday Mar 18, 2016 at 22:11 GMT


It complicates things a little for people that maintain and do things in their own branch. But that statement is true of any change we make. I think what we would do is try to make the minimal number of changes to slice the code apart. That would make it much easier for somebody to cross their changes over to the new code base. And it is from this foundation that we would try to continue the development work.

I think the topic deserves discussion to make sure we hear all of the concerns and get the best thinking out on the table. After the discussion we will do our best to minimize the impact on people trying to maintain their own forks. But we really do need to start cleaning up the code. Marlin_main is over 5000 lines long just because we could not slice it up and use the current tool set.

boelle commented 8 years ago

Comment by Roxy-3DPrintBoard Saturday Mar 19, 2016 at 13:45 GMT


I said:

If we went this route, that is probably very easy for the user. When they upgrade to a version of the firmware that is organized around the new file layout, they would do what they always do. They would do a diff on their configuration.h file and compare it to the new configuration.h file to see if there are any new settings they need to cross over.

and:

I think what we would do is try to make the minimal number of changes to slice the code apart. That would make it much easier for somebody to cross their changes over to the new code base. And it is from this foundation that we would try to continue the development work.

What part of trying to make it as easy as possible for the user don't you understand?

But here is something for YOU to understand: The developers here literally have put in 1000's of hours of effort to design, write, debug and maintain the code. The code base desperately needs to be cleaned up so that it can be reliable and easier to continue development. If you don't want that, you can just stick with an old version you know and love. If you want the code base to evolve, you are going to have to spend an extra 15 or 20 minutes when you switch to the latest and greatest version the first time.

If we are going to make the code better, there have to be changes made to it. It isn't realistic to say "I want to the code to be better but you are not allowed to make any changes."

boelle commented 8 years ago

Comment by Roxy-3DPrintBoard Saturday Mar 19, 2016 at 17:57 GMT


I apologize. I took your message to mean you had some changes you added to your fork and you were not willing to spend a few minutes to move to an updated code base.

It depends on which developer you ask, but pretty much they are all in agreement the code base needs to be cleaned up. Different issues are affecting different things. The size of some of the files is causing major issues with scoping right now. There is a very real desire by other developers to shift to a class based hierarchy. The size of files and not having functionality grouped together is complicating that shift.

The fact we are not modulized and some of the files have grown to huge sizes also makes it difficult to stay out of each other's way when there are multiple areas being worked on simultaneously.

boelle commented 8 years ago

Comment by Roxy-3DPrintBoard Saturday Mar 19, 2016 at 19:28 GMT


That's kind of the part I'm missing, the way people are getting trounced upon. The github system handles the merges. There would be interactions when "they unfortunately picked the same spot" to add code, or changed the same content. Moving to objects "might" prevent both of these, but probabilistically only the first.

In theory, Yes. But in practice it doesn't. A good example would be the Auto Bed Leveling code. Currently, we have None, 3-Point, Mesh, and Grid. Each of these has various flavors and targets. When somebody changes something simple like what happens when the Z-Probe is retracted but still triggered, chaos breaks out. You start to layer different machine types on top of the Auto Bed Leveling and you really have a complicated stew to pick through. And then you add the fact nobody has each machine type and it is real easy to understand how problems creep in.

The code needs to be simplified and cleaned up.

If you jump to use the big tools/ideas, the product will end up with the fully protected execution environment. Bang, 8 bits may be too slow. (I have no concern one way or the other.)

If I were a business, I would look to the future and see if Marlin fits into the rollout of those cheap, slow 4.3GHz SOC 8 core 64b processors while maintaining dominant presence in the current low cost, hobbyist world.

We are pretty careful to keep the code efficient. I think the 8-Bit AVR's still have some life left in them. But it really doesn't matter. The high performance 64-Bit AVR's are starting to be available and will be very inexpensive soon. There are already Marlin ports to the 64-Bit AVR's. We probably will be able to support both with the same code base for some time. It may be some of the fancier stuff like full graphics LCD Panels only run well on the 64-Bit AVR's. I don't know what compromises we will have to make, but I'm pretty sure we can adjust.

We already have some use of C++ classes in Marlin. They haven't been too hard on the processor. But we will have to be careful about that as we introduce more class hierarchies.

boelle commented 8 years ago

Comment by jbrazio Sunday Mar 20, 2016 at 20:27 GMT


The problem with objects is I believe it's incompatible with the current Marlin dev workflow. Objects would require everything to be designed and planned upfront implementation, this is not the current Marlin philosophy.

boelle commented 8 years ago

Comment by Roxy-3DPrintBoard Sunday Mar 20, 2016 at 20:43 GMT


Wow!

Perhaps a counter example will help the discussion: Would a Z-Probe object require everything to be redesigned prior to it's instantiation? My guess is a Z-Probe object could be made that supported things such as Deployment(), Retraction(), Run_Z_Probe(), Report_Repeatability(), Set_Servo_Angles(), is_triggered() etc. without redesigning everything else.

And if this Z-Probe object existed, it would be almost trivial to use it instead of the existing calls. As @gddeen suggested, the Grid, Mesh and 3-Point Auto Bed Leveling have a lot of common usage of Planes in their functionality. This is more tricky because the real time code needs fast and easy access to underlying data. We would have to be especially careful how we constructed an Auto_Bed_Leveling object for this reason. But I think we can see a few methods and class data that all flavors have.

boelle commented 8 years ago

Comment by thinkyhead Tuesday Mar 22, 2016 at 05:35 GMT


Stepper, planner, temperature, and other sub-modules can today quite easily be wrapped up into singleton classes to get the encapsulation we like, and without any performance hit, because we would not be using any fancy features of C++. This is a process that will unfold gradually. The functionality of these units and the contents of the functions of the initial classes would follow the same pattern as the current code. But, as time goes on and more things –like endstops, extruders, etc.– become encapsulated into objects, the structure will begin to fall into place more. I don't think we'll see any major overhaul of the execution flow. But we will have nicer encapsulation and the code will be getting easier to follow over time.

boelle commented 8 years ago

Comment by gddeen Tuesday Mar 22, 2016 at 14:14 GMT


jbrazio. There are currently "objects" in Marlin. It's simply c++. One does not need to think and plan to create/design objects. However, objects can inherit "stuff" that just magically changes their behavior. Maybe that isn't the smartest thing to do on an 8bit computer. So, don't do that.

Classes, objects all sorts of programming concepts were added in 80s for good reasons. Hacking (Making) will still survive.

boelle commented 8 years ago

Comment by CONSULitAS Friday Mar 25, 2016 at 23:22 GMT


@gddeen Perhaps you should think and have a look at examples for objects on Arduino before you disencourage people here by nonsense. Objects are not slow on an 8 bit machine! Stupid programming is slow.

If you don't believe i recommend some reading for you: http://cosa-arduino.blogspot.com.es http://cosa-arduino.blogspot.com.es/2013/03/benchmarking-pin-classes.html (slow objects?) https://github.com/mikaelpatel/Cosa http://forum.arduino.cc/index.php/topic,150299.0.html

Did you read it all?

So perhaps start contributing good code and show working concepts instead of bashing an important approach for making Marlin more maintainable and even more flexible.

BTW: I read it...

boelle commented 8 years ago

Comment by jbrazio Saturday Mar 26, 2016 at 02:01 GMT


I'm not saying we shouldn't move to objects, it's a pleasure to read @repetier code. I'm just saying the with the technical shift a mindset shift must also happen.

A quick illustration of what I mean: a key feature of good OOP is self containment, meaning [as an example] global vars are over, if you need to access a particular data struct then this must be published by the object's public interface and no form of "hacking" should be tolerated from the developers. There is no more coordinate changes by just editing the current position array and then informing the planner.. we must tell the planner our intent and then it's up to it to do it and then inform us of the result.

boelle commented 8 years ago

Comment by Sniffle Sunday Mar 27, 2016 at 04:51 GMT


Damn wheres a like button when you need it...

If your gonna go OOP do it right or not at all, maintain scope and keep things private between objects so that the objects are just little black boxes that take inout and give output. I kinda have a feeling this is the turning point for marlin. Its either gonna grow back into the monster powerhouse firmware it once was or its gonna fall into obsolescence. This is basically the step that allows marlins features to move forward onto 32 and 64 bit chips. I realize im a very small player in this but you guys also know i only open my mouth when i have a good point to make.

boelle commented 8 years ago

Comment by CONSULitAS Sunday Mar 27, 2016 at 13:15 GMT


@Sniffle It is new just in the right upper corner of any comment shown as + :smile: .

boelle commented 8 years ago

Comment by Sniffle Sunday Mar 27, 2016 at 17:21 GMT


<3 roxy i just saw something worth talking about and threw in my .02 like usual :-D

boelle commented 8 years ago

Comment by jbrazio Sunday Mar 27, 2016 at 18:14 GMT


What happen to @Roxy-3DPrintBoard comment ?

I'm pretty sure this object would be a global variable of class Probe

Of course we still have "global" pointers to objects, but they are no longer global, they are inside the Controller class.. in a nutshell what we know as Marlin_main will be a Marlin controller class which will take care of instancing all the other objects.

boelle commented 8 years ago

Comment by Roxy-3DPrintBoard Sunday Mar 27, 2016 at 18:53 GMT


What happen to @Roxy-3DPrintBoard comment ?

I deleted it because I'm probably the only one that doesn't have a formal Computer Science training. I figured I would end up getting picked on because there is probably some technicality where an object is not considered a variable or some such non-sense! :)

boelle commented 8 years ago

Comment by Sniffle Sunday Mar 27, 2016 at 18:55 GMT


I only partially completed mine so im only good for theory. I do understand thats why i dont talk much.

boelle commented 8 years ago

Comment by ghumphr1 Sunday Mar 27, 2016 at 19:06 GMT


@Roxy-3DPrintBoard - I am an engineer - and i think you should have say whatever.... just because you havent such and such piece of paper doesnt mean you have no valid input.. more so than some of the 'qualified' people most times

boelle commented 8 years ago

Comment by Roxy-3DPrintBoard Sunday Mar 27, 2016 at 19:14 GMT


and i think you should have say whatever....

Everything is good. And if you saw the deleted comment, the first line of it was "These types of discussions tend to turn religious..." That is because the object orientated topic is very important with very strong beliefs on all sides.

What we are all after is a cleaner and more easily maintained code base. And probably some C++ objects can help on that topic. But for me it isn't a religious matter. It is about putting the objects in places where it makes sense to do that and helps encapsulate data and functionality.

Because Marlin has a bunch of real time code and is currently powered by a slightly under powered AVR processor, we have to be careful how we do it. We can move in that direction, but it has to be carefully thought out.

boelle commented 8 years ago

Comment by jbrazio Sunday Mar 27, 2016 at 19:18 GMT


As long as we don't go into the malloc() road..

If you go object oriented.. you should go all the way; the problem is when people forget they are on a AVR arch and go all over the place doing "dynamic stuff".

boelle commented 8 years ago

Comment by ghumphr1 Sunday Mar 27, 2016 at 19:18 GMT


boelle commented 8 years ago

Comment by Roxy-3DPrintBoard Sunday Mar 27, 2016 at 19:49 GMT


As long as we don't go into the malloc() road..

Another religious topic when it comes to firmware! malloc() and free() can be used safely but they are so controversial, it is just best to avoid them if possible.

maukcc commented 8 years ago

Let me see if I get this straight (and put some fuel on this fire) : ) Marlin is being developed in a horizontal way and not an upwards way because some users can not be bothered to update to the latest Arduino IDE (or some other silly thing)? I for one love the KIMBRA approach as it lets me install a new firmware in 5min in stead of 1day because all of my machine specific changes to the firmware are in 1 place.(I just have to copy that 1 file)(and I am not new to this, but I forget I changed some silly things in files that are not configuration.h) So: let users conform to the firmware, and not conform the firmware to users that can/will not keep up with the future. It is "rapid" prototyping after all. I think this is also the reason why a lot of users have switched to Repetier or Kimbra. I would have switched as well if it wasn't for the poor DOGM lcd support.

jbrazio commented 8 years ago

You can't compare.. The reason why "moving one file" does not work lately is indeed a signal of heavy active development.

maukcc commented 8 years ago

Unfortunately the heavy active development is done mostly in horizontal plane (getting more robust firmware) and that is very good. Nothing wrong with that (I changed to RC6 the other day and can feel the changes). But the upwards pane (new features) is being set to the background compared to other firmwares. And that needs to be addressed as well, as new (and old) users see flashy new/easy things over at the other side.

thinkyhead commented 8 years ago

@maukcc Glad you like the MarlinKimbra approach. We do too, which is why we will be emulating that fork's arrangement (generally) in the upcoming development branch for 1.2.x.

jbrazio commented 8 years ago

Follow up the split on the branch: https://github.com/MarlinFirmware/Marlin/tree/breakup-marlin-idea

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.