Marlin 🚀 - file(s) to write down things (at least new) developers should have an eye upon?

I think those types of things are difficult to corral, but I'm sure if you had an addition to this, it wouldn't be turned away ! :) http://marlinfw.org/meta/development/

fiveangle on 5 Dec 2017

I second the referral to MarlinDocumentation. I've been thinking since I started participating here that it would be really nice to have more robust documentation for new developers - things like project structure and etc - documented along with the more specific bits like coding standards, collaboration process, and specific pitfalls like this.

I think it would also be really useful to have some kind of meta/overview document on the 1.1.x vs 2.0.x development roadmap. E.g. as a (relatively) new user, and completely new contributor, is it more useful for me to be running and testing bugfix-1.1.x or bugfix-2.0.x (on an 8-bit platform)? More useful from the perspective of giving feedback to the project, since I've decided to run a bugfix release. Or, is bugfix-2.0.x generally stable enough to run on a daily use machine at this point?

I would volunteer to start writing some of these - I like documentation - but at this point those are questions that I still need to understand so I can't really answer them yet.

bjarchi on 5 Dec 2017

Ok, the fact I didn't know this place, tells it all :-) (I mean the real "Coding Standards" part)

Yes, it's definitely my fault...I apologize...
But I think, in this regard I'm a typical developer.

A lengthy document that starts with "Adding new Fonts" and a Coding Standards section that starts with "Coding style" doesn't promise to give me the essential information I really need.

All this information is useful and yes, I am glad, it is there and I promise to read it.

But I think it's not very realistic.
If you are chasing a problem with your printer and then start looking at the code, you never ever would read such a big document, first. You are in the code base.

But the importance of the lower portion of the Coding Standards section seems to be very low.
I think, this is the part, that may prevent long bug sessions, because it contains some essential parts.
And this part has only about 5 pages...

I would suggest to place this directly in the code, where most developers will probably see it.
Also coding style could get it's own text file as a reference.
However, I would link from those files to the web side and vice versa.

Some experiences from 30 years as a developer:

Never separate the documentation from the code. Then it simply doesn't exist.
While digging in the code, you will easily find interesting files here and there. And you read them.
If you search for something (e.g. a macro named COPY), you will usually also find results in documentation files, if the search words are mentioned there. If you then see some warnings around these search results, you will probably start to read.
This is why these documentation files should include all the words you may search for.
The information should be precise, dense and true! Most developers are used to work fast and don't have time to read nice words.
Do not mix documents for users and for developers. Most developers will stop reading a document addressing users.

Let's look at some files in the repo:

README.md is essential and every developer will probably read it. This one addresses developers even if it will also be read by most of the users, I guess. Despite it addresses developers there is no single word of the coding standards. Section "Contributing to Marlin" should contain a link.
README.h is a source file and I guess most users will not read it. But it literally addresses users "Greetings! Thank you for choosing Marlin 2 as your 3D printer firmware."
"Marlin Firmware Official Website", "Configuration", "Getting Help" are all for users.
Here is a small section at the end (where most developers may already have left the building) called "Contributing" containing the important sentence "Before submitting code get to know the Coding Standards."

"Coding Standards" is probably correct. But it is not sexy.
That's because every link that refers to coding standards always starts with the style.
Now, as an experienced developer you are usually quite able to derive the style from the code much faster than reading such a document. You don't need this.
From my experience a document like this usually doesn't have more information at all. So you fade away.
The page "Coding Standards" is about half about style. You do not expect to see something really useful after it.

Marlin is a bit different, because most users have to deal with developer tools and sources.
There is no real separation between users and contributors. Additionally users can morph into developers over time. So the reader chooses what to be.

some suggestions:

general

separate user topics from developer topics
sort sections by importance not by alphabet. If there are too many, use groups or subsections.
don't duplicate information

website

at least split this section into "Development Standards" and "Coding Style" and put the standards first
move "Development Standards" to the source. The web page may be generated from it, but a link would be enough.
I personally would also move "Coding Style" to the source. I still don't think that many developers will have a look on the web page. Would be nice to have some statistical data relating github contributors and users of these web pages.

source

add searchable files with the important development information (not the blurb)
make them easily accessible, but clearly mark them as developer/contributor files

hg42 on 5 Dec 2017

With respect...

But I think it's not very realistic.
If you are chasing a problem with your printer and then start looking at the code, you never ever would read such a big document, first. You are in the code base.

I think that if you're starting to contribute to a large, distributed, collaborative project like this then it's sort of a responsibility to at least skim over the developer documentation since those things provide a framework for how people work together on the project... although I think you're right that a lot of people don't do so.

"Coding Standards" is probably correct. But it is not sexy.
That's because every link that refers to coding standards always starts with the style.
Now, as an experienced developer you are usually quite able to derive the style from the code much faster than reading such a document. You don't need this.

It may not be sexy, but it is critical to collaborative development - and that includes coding style. Failure to follow code style guidelines creates huge headaches when looking at diffs, handling merges, etc. And I'm pretty sure all that responsibility falls on @thinkyhead (and maybe a few others) right now.

An experienced developer should be able to derive most of the coding standards from existing code, but looking through this project's PRs suggests that many people still don't... and there are probably things in the standards that aren't completely obvious from the code.

As for the documentation site, I personally think it's much more complete and better structured than most free software projects - this is a huge plus for Marlin. Personally, I would say that the documentation site also does a good job of separating user and developer documents... I would just like to see more of the 'intro' developer docs for things like project roadmap.

I agree with a lot of your suggestions, @hg42, but I think you're being a little dismissive of the responsibility that new developers have (should have?) for conforming to established norms of process, and perhaps of how much work that makes for the main developers.

Maybe it would be helpful to have a file that links to the doc site and banners new contributors to read the coding standards?

As for other suggestions re: the developer site, it's all on github too (MarlinFirmware/MarlinDocumentation) - it's quite easy to get a build environment set up to start working on the docs. If you have more experience with the project than I do, maybe make some of those changes you suggest?

bjarchi on 5 Dec 2017

Far out. This does work…

#define COPY(a,b) do{ static_assert(COUNT(a) > 1 && COUNT(b) > 1, "COPY macro error"); \
                      memcpy(a,b,min(sizeof(a),sizeof(b))); }while(0)

thinkyhead on 5 Dec 2017

@thinkyhead I thought about the assert in the macro, too.
Two problems:

fails for non-arrays (and nothing says it's for arrays only)
fails if array has only one element (which seems to be non-sense but may exist)

hg42 on 5 Dec 2017

@bjarchi

if you're starting to contribute to a large, distributed, collaborative project like this then it's sort of a responsibility to at least skim over the developer documentation

yes, I agree about the responsibility, but it's also not realistic from my experience.
In the first place, It's not the place where a developer would search for the guts.

And one of my important points is, that even if you look for it (like I did!), you see a lot of information, but you simply don't see the essential ones. They are kind of hidden.

At least a pointer from the README.md directly to the "Coding Standard" section would already help.
This shouldn't be at the end of the Readme.h like the (general) documentation link now.
Perhaps the file your looking at when opening the project (Marlin.ino) should contain this link at the top.

A user is in two modes, when opening the project, either configuring (so you need a pointer to configuration.h) or coding or finding a bug (then you need a link to the main reference information).
Regular developers don't have this problem.

But, my main point of all I wrote is, that the reference information is not included in code searches.
Note, MarlinDocumentation is in a separate repo.

since those things provide a framework for how people work together on the project... although I think you're right that a lot of people don't do so.

So we agree, it is very important?
My conclusion is I shouldn't have to dig for it.

It may not be sexy, but it is critical to collaborative development - and that includes coding style

I meant, "Coding Standards" is somewhat devalued by the usual use of it.
There are millions of developers that don't like to document (and I have a bit of understanding for them, because apparently no one reads these documents).
If they write such documents, they almost always start with the easiest topic, which is talking about style. Doing this, you stop the reader from reading further, because style is a fairly controversial topic, which everyone loves to disagree and debate about.

I totally agree, style is mandatory and should be somewhere in front of the developer material.
But NOT in front of the more essential reference part, which is the other conventions and rules etc.

Real reference information is much more difficult to write and often does not exist. So from the layout of the documentation I would not expect to find it in Coding Standards. At least this should be a separate Section.

I also agree, Marlin documentation is very good in comparision.

But the reference information is still buried and not part of code searches.

hg42 on 5 Dec 2017

thinkyhead : Far out. This does work…

The macromaster strikes again ! 😝

There are millions of developers that don't like to document

hg42 : If they write such documents, they almost always start with the easiest topic, which is talking about style. Doing this, you stop the reader from reading further, because style is a fairly controversial topic

That's a great perspective I hadn't considered !

@thinkyhead - how much effort would it be to expound on the existing docs with a higher level "Marlin Structure Reference" doc that gives a framework to how things generally work ?

It's too bad there wasn't something simple like a MediaWiki that could be easily added to real-time by anyone (the beauty of which any change is easily reverted with the only downside being the potential for short periods where bad info is present, which is a totally reasonable trade off compared to the high barrier of entry to updating the current docs). I know I'd have added countless little "FYI"s based on my tinkerings. For the actual seasoned devs, it would be a place to direct those with the countless regurgitated questions to, instead of trying to link them to the manifold of disjointed discussions on the issue tracker. One can dream I guess... ;)

-=dave

fiveangle on 6 Dec 2017

@fiveangle unfortunately the ideal isn't the reality.
I think it's a game of guiding the masses to the information they want or need.

The game starts randomly where the "user" jumps in. This may be some Google search result or after cloning the repo.
So there are many entry points.

Now you have to guide them by giving them the right buzz words.
"Printer Configuration" go there...
"Bugs" go there...
"Suggestions" go there...

At some point you have only the kind of information this kind of user needs.
There can be little pointers like "if you want to add some feature then go there..."

According to the wiki, that would indeed be a good tool.
But only for user related themes.

Developers are different (everyone knows, right? especially REAL devs).

They usually don't add to wikis if they are not forced to do.
There usually is someone (called a victim) that writes a page like the developer info in the Marlin docs and after this there is no update at all. From my experience this doesn't change with wikis.
Example:
COPY macro isn't referenced in that document at all. And this should be a quite basic macro.
fastio only references READ and WRITE, what about TOGGLE, and 8 other main functions?

If you'd ask me, macros.h would be the place to document macros and fastio.h would be the place to document these (in this case there should be a base file somewhere because it is different in each HAL).

If someone searches for a macro, the search result will probably include macros.h
And once you have seen there is a macros.h you will look what's available there. Why not have information about the purpose? especially if the docs only have one short line for each...
Even if there are no comments, you will not try to search online, right?

Also, nobody keeps documentation and code in sync. As a developer I learned, you should never look in the documentation first, because it often leads to totally wrong assumptions and conclusions, because it is almost always outdated.
Instead start with the code and if you look at the docs, always be aware of outdated information.

hg42 on 6 Dec 2017

I suppose I've worked in enterprise software development too long. Any dev working on a project too large for themselves to fully understand (pretty much every project at my co.) knows that if they don't document their code as they write it, either with adequate intelligent comments, or augmenting the wiki (auto-linked in header of each source file), they will have to field all questions in perpetuity from the last large commit in ClearCase. Manager buy-in to fully support this is what makes it work, so yeah, not as likely with a hodge-podge of "hacks" and "brogrammers" who just want to dump and run their latest contribution that gets what they want done.

Can't please everyone 🤷‍♂️

fiveangle on 6 Dec 2017

macros...
I think instead of including static_assert in macros (which indeed works), it would be even better to use templates instead of the macro, because they only apply to matching types.
If the types don't match you will get an error.

E.g. the COUNT macro could be something like this (copied from www, without testing):

template<typename T, size_t n>
size_t count(const T (&)[n]) {
    return n;
}

this cannot be applied to types where n isn't known by the compiler.

The same applies to COPY, even if it uses memcpy (which may create some headache for general cases, that's why I would not call it simply COPY):

template<typename T, size_t n, size_t nfrom>
void copy(T (&to)[n], const T (&from)[nfrom]) {
    memcpy((void*)&to, (void*)&from, min(n, nfrom));
}

You could also define copy for other types, if you want:

template<typename T>
void copy(uint8& to, const uint8& from) {
    memcpy((void*)to, (void*)from, min(sizeof(n), sizeof(nfrom)));
}

so you can implement different copy functions for different types without bothering the user with different names (polymorphic).

hg42 on 6 Dec 2017

Any dev ... knows that if they don't document their code as they write it, either with adequate intelligent comments, or augmenting the wiki (auto-linked in header of each source file)

that's what I want. And if you don't have another working infrastructure you should do it in the code base. It's the way everyone understands. And ir's where every dev would expect such things at first.

Believe me, I'm used to document, but I'm also used to useless documentation.
We are not discussing whether to document or not, but where and how and which is most effective.

In the first place, I only want some files to add crucial information.

Btw. @thinkyhead is on the right track (assertions). Because it's even better to prevent a problem than documenting how to prevent it.

hg42 on 6 Dec 2017

👍1

I meant, "Coding Standards" is somewhat devalued by the usual use of it.
There are millions of developers that don't like to document (and I have a bit of understanding for them, because apparently no one reads these documents).
If they write such documents, they almost always start with the easiest topic, which is talking about style. Doing this, you stop the reader from reading further, because style is a fairly controversial topic, which everyone loves to disagree and debate about

So we agree, it is very important?
My conclusion is I shouldn't have to dig for it.

I guess we are in agreement then :)

It sounds like you have more perspective on what 'normal developers' / software engineers do - I'm more of a scientist/engineer who happens to write a lot of code (although that is currently most of my job). Maybe we can work together on cleaning up some of the documentation site side of the developer docs, since I've already done some work on the documentation?

bjarchi on 6 Dec 2017

I still don't get why comments like this are not directly in the serial.h where everyone needs them?:

Function|Purpose
--|--
SERIAL_ECHO_START() | Send “echo:” to the serial output.
SERIAL_ERROR_START() | Print “error:” to the serial output.
SERIAL_PROTOCOL("hello") | Print an ASCII string stored in SRAM to serial out.
SERIAL_PROTOCOLLN("hello") | Print an ASCII string stored in SRAM to serial out, appending a newline.
SERIAL_PROTOCOLPGM("hello") | Wrap the given ASCII string in PSTR and print it to serial out.
SERIAL_PROTOCOLLNPGM("hello") | Wrap the given ASCII string in PSTR and print it to serial out, appending a newline.
SERIAL_PROTOCOLPAIR("Hello:",val) | Wrap an ASCII string in PSTR; print it and a value to serial out.
SERIAL_PROTOCOLLNPAIR("Hello:",val) | Wrap an ASCII string in PSTR; print it, a value, and a newline to serial out.

hg42 on 6 Dec 2017

@thinkyhead I wanted to see if such a COPY template would work and I got this:

#if defined(__cplusplus)
  #include <stdlib.h>
  #include <string.h>
  template<typename T>
  inline const T& min (const T& a, const T& b) {
    return !(b<a)?a:b;
  }
  template<typename T, size_t n, size_t nfrom>
  inline void COPY(T (&to)[n], const T (&from)[nfrom]) {
    memcpy((void*)&to, (void*)&from, min(n, nfrom));
  }
#else
  #error no COPY macro defined for C files
  // or enable this
  //#define COPY(a,b) memcpy(a,b,min(sizeof(a),sizeof(b)))
#endif

if used with a simple array function parameter without reference in _buffer_steps, you get this message:
Marlin/src/module/planner.cpp:1385:24: error: no matching function for call to 'COPY(int32_t [4], const int32_t*&)'

hg42 on 7 Dec 2017

I wanted to see if such a COPY template would work and I got this:

Fancy! When I added COPY I was just tired of typing out the code to copy arrays.

If every macro gets the C++ template treatment I worry it could get ugly, but go ahead and give it a try. I have no doubt that such an approach is more pro, probably adds no overhead (given the smartness that gcc has these days), and will probably catch more common errors.

If you feel like producing a suite of these to replace some common macros —and can demonstrate that they produce code that is no larger or slower than current— then I'm sure we would adopt it.

thinkyhead on 7 Dec 2017

When I added COPY I was just tired of typing out the code to copy arrays.

I do this often, but it's dangerous, as you saw, because you hide implementation details (which is good otherwise). The macro would have to be called something like COPY_ARRAY_USING_SIZEOF to remind the reader of the pitfall. But anyways not many expect this, even knowing it's based on sizeof. To be honest I also wasn't fully aware of this, and had to see it myself in the debugging output.
For me it's another damn decision of the C++ commitee.

If every macro gets the C++ template treatment I worry it could get ugly

yes, I am sure it does.
To use templates the code generally needs to be more strict. The types have to be clean (things like const etc.). However, I think this would be worth the trouble, because the code will get more stable.

But honestly, I don't think it would be easy for Marlin. This code base is very C centric. I don't see a chance to get it really clean. I bet a complete rewrite would be faster. Perhaps this could be a task for Marlin 3.

Also, someone with good C++ knowledge would have to maintain the set of templates.

That said, I would probably do this, BUT..

hg42 on 7 Dec 2017

As you know I am quite a new Marlin contributor.

Some days ago, when I came back to the Marlin repo after several years, I was actually looking for possible firmware for my Smoothieboard and wanted to check how difficult it is to run Marlin on it.
I was quite impressed how easy it was. I think I entered just in the right moment.

In general I am searching for a future combination of a board and firmware to fit my future needs.
Because the software is a key part in this game, I started examining them.
I am quite sure, I can only decide which firmware to use next, if I get my hands into it's guts.

In case of Marlin 2, this was right on my first contact. When I configuring my printer, I just had to solve a simple but virulent bug for the LPC1768.

In the last days while being active on the repo I discussed some of these topics with my best friend, who also uses a smoothie compatible board.

Marlin is deeply based in the 8bit world. I changed to 32bit some years ago, because I used a delta and it's speed and quality were limited by the limitations of the 8bit hardware. Today 32bit boards are not much more expensive than 8bit boards.
There is no point for me to stick again to 8bit limitations, even if it only influences the 32bit part.

My current goal is to break free from any limitations, that are slowing down innovations for my printing needs.
I came to Marlin because I think, it is more open and gathers more innovative power than any other firmware, and I am still convinced it is. I really like the community and it's drive.
Marlin has a set of features I wanted to explore, like LinAdvance.

On the other hand, I want to use a clean and modern code base to build upon.

It turned out, Marlin is not the best path for me.
In fact I currently don't see any perfect one for me.
So I will probably stick to Smoothieware for now. I used it for some years now and I am familiar with it.

I guess, in the near future I will probably explore other ways.
E..g. I am thinking about a modular hardware approach.
May be using a Pi controlling several external modules.
A module could be something like an Arduino nano for each axis including endstops and probes.
Another module could control a heater including thermistor and fan.

So, I have to say bye to everyone...it was nice to meet you all.
Have a good time...

hg42 on 7 Dec 2017

👍1

Welp... close?

Marlin is deeply based in the 8bit world. I changed to 32bit some years ago, because I used a delta and it's speed and quality were limited by the limitations of the 8bit hardware. Today 32bit boards are not much more expensive than 8bit boards.
There is no point for me to stick again to 8bit limitations, even if it only influences the 32bit part.

I have to say this is somewhat of a concern for me, too. I expect I'll go the 32-bit route at some point soon, for performance (higher µ-stepping esp.) and because I have some (small) experience with ARM development, and I do wonder if keeping a single code base for 8- and 32- bit platforms will slow down the full exploitation of the 32-bit controller advantages. That said, this is probably something for a higher-level development discussion rather than a bug report :)

I guess, in the near future I will probably explore other ways.
E..g. I am thinking about a modular hardware approach.
May be using a Pi controlling several external modules.
A module could be something like an Arduino nano for each axis including endstops and probes.
Another module could control a heater including thermistor and fan.

This is a really interesting approach. I hope you pursue it!

bjarchi on 7 Dec 2017

and I do wonder if keeping a single code base for 8- and 32- bit platforms will slow down the full exploitation of the 32-bit controller advantages. That said, this is probably something for a higher-level development discussion rather than a bug report :)

At some point it won't make sense to keep a single code base. But for right now, we would like the starting 32-bit code base to have the full feature set of the 8-bit code base. When there are things that can not be done for both, we can fork the code base if necessary.

The way Marlin is setup with conditional compilation make it so that point in time is probably a ways out there.

Roxy-3D on 8 Dec 2017

I do wonder if keeping a single code base for 8- and 32- bit platforms will slow down the full exploitation of the 32-bit controller advantages.

Conversely, I wonder if a 32-bit rewrite could be as feature-ful and support as broad a range of hardware as Marlin's initial 32-bit port?

As Roxy points out, our idea is to expand on the work already done on Marlin for DUE and to migrate the code-base to an abstraction layer, with better separation of the high-level and low level code. We envision Marlin's high level code being increasingly portable, while the low-level code will be more modular. This exercise is forcing us to sift and sort the code and to refine its design as we go. Sometimes it does feel like we're writing an AVR emulator on which to run Marlin, but that's not a bad thing _per se_. In the long run we're going to have some very useful glue code on which to build future systems.

A shiny new firmware codebase (e.g., Smoothieware) is always going to be more fun for those who just want to get down writing high-level C++. Marlin has been a lot of drudgery, undergoing a slow evolution over the last few years, which continues to mean a lot of tedious refactoring — all in the midst of maintaining a working code-base, incorporating features, and keeping up with the Joneses and the Prusas. Marlin is not unlike a city infrastructure, built on the skeleton of the older system.

So, I have to say bye to everyone...it was nice to meet you all.

I hope you will come around and share tales of your discoveries. Have fun!

thinkyhead on 8 Dec 2017

Hi, nice that you answered...

May be some of my words were a bit too strong. I didn't want to offend anyone.

To be clear, I highly respect what you all are doing here. Really.
You are doing a great job here.
And I enjoyed to be and work with you all for some time.

I know the history of Marlin and I like how it is developing.
Especially I like that you have a HAL. And it already works. Interestingly most other firmwares don't have this or not complete.
It's amazing on how many targets Marlin can run. It was a great experience to be able to bring it on the Smoothieboard with only a few defines.

I also know, how difficult it is to evolve such a beast and keep it on top.

I did this kind of job for a living for a long time. You cannot or do not want to throw away all the features (like Mozilla is unfortunately doing on firefox currently, dropping all it's add-ons and many developers).
You do not want to start from zero. Instead you have to intelligently find a migration path.

I see you are on the right track by making Marlin more modular. At a point you have a clear module definition for something (in a more abstract sense, not necessarily a C++ module, just something you can plug together, so functionality and interface are stable). Then you can start to replace implementations. At this point it doesn't hurt, if you change something, because you always can use the old module. It's also easy to compare the two variants. You get a kind of competition.
In theory everyone could write a new implementation and offer it to the community.

When I said

It turned out, Marlin is not the best path for me

the words for me were very significant. I should better have emphasized them.
It just happens that the current redesign process of Marlin doesn't fit very well to my upcoming needs.
I need a more simple and stable base to build my own experiments upon.
Marlin on the other hand is in total flux. More like a moving target.

Thanks for some great days...have a good time everyone.
May be I come back at some point. I'll definitely have an eye on the development here.

hg42 on 8 Dec 2017

👍1

Marlin: file(s) to write down things (at least new) developers should have an eye upon?

All 22 comments

Related issues