I have decided to go an easier way to get my cnc-mill up and running. For propeller-purists a deadly-sin.
I just bought an arduino clone for $15 a three-axis 3,5A driverboard for $30 loaded the newest GBRL firmware up to the Ardi-clone connected wires and are up and running.
The whole CAD-CAM-toolchain I use is freeware:
Just for testing I made a sphere in google-sketchup cutted the pole of the sphere out. Exported to dae-fileformat. Converted *.dae with MeshLAB to STL. Imported STL-File into ESTLCAM
which generated the gcode. Then used filemode of universal-gcode-sender to send the whole gcode-file down to the arduino-board and milled the part of the sphere out of a massive block of wood.
Wow! never had such a straight forward success with software before. So for me personal I'm having my cnc-mill up and running without a propeller-chip.
P.S.: It was only me who was pointing on DON STARKEYS propeller-CNC-code in the OBEX. I tried it and had problems to get it running.
I will still do smaller projects with the propeller-chip but not a CNC-interpreter.
@Stephan38
Hmmm. I got you mixed up with Don Starkey. He provided the CNC code; you revived interest in it.
===========================
I certainly realize that the combination of existing boards and mature code out there for the AVRs make it easiest to just open your checkbook and buy all the items that work.
In my own case, I am wanting to learn more about the Propeller. So, the availability of Adruino code to port to the Propeller seemed a logical way to get up to speed.
Tonokip_with_Serial seems to have been provided in a state where the code was being debugged and incomplete. It is not a very good place to seek out a working port. But I keep fiddling with it to sharpen my skills. Also, the serial port configuration seems to not be right.
I started to port Teacup, but ran into code that provides selection for a wide variety of Arduino chips. Also it appears to be for OpenAVR, not really Arduino specific. So that created quite a few additional topics to think through... even though that code may actually work properly and well on an Arduino.
And then GRBL, for non-3D printer CNC projects seems to be the clearest of these 3 choices for porting. But it will not provide the additions need for a filament printer head and bed heater. So in spite of it being the most likable code to begin with, it leads only so far.
What am I trying to say? I guess I am just explaining why I have so little to show at this point. Someone else, with more experience with C and C++ may have gotten this all resolved by now. But I am not getting any volunteers to lead the way.
I keep plodding on as I am enjoying finally learning to use C and C++ on the Propeller. There is a faster track if you don't want to learn what the actual code is all about.
+++++++++++++
In all honesty, a Propeller 2 or Propeller 1+ chip might really make this project move ahead with a much more feasible, full-featured 3D printer g-code interpreter. Space has been an issue all along.
I suppose you would have to figure out what move_state.endstop and dda->n are supposed to do and then take a look at what the tests want to do it. They appear to be process flags for the run of one interpolated G1 call.
It seems that if the G1 has run its complete series of x, y, z, and e steps; the code is support to consider that one G1 sequence complete. Acceleration and deceleration are ramped up and brought down with each and every G1 call.
In other words, this just appears to clean up the the G1 and make ready for the next G1 run if Acceleration Ramping is implemented.
I have been studying the GRBL CNC code to better understand the acceleration ramping and Teacup seems to not explain all the details.
+++++++++++++++++++++++++++
I find is very odd and confusing that the thread title can be hijacked by any poster. I didn't know that. I have returned it to the original.
Kind of a long way to go about making it easier to understand, but I rewrote it. Does this look right to you guys?
Original:
// If there are no steps left or an endstop stop happened, we have finished.
if ((move_state.x_steps == 0 && move_state.y_steps == 0 &&
move_state.z_steps == 0 && move_state.e_steps == 0)
#ifdef ACCELERATION_RAMPING
|| (move_state.endstop_stop && dda->n == 0)
#endif
) {
dda->live = 0;
dda->done = 1;
#ifdef LOOKAHEAD
// If look-ahead was using this move, it could have missed our activation:
// make sure the ids do not match.
dda->id--;
#endif
#ifdef DC_EXTRUDER
heater_set(DC_EXTRUDER, 0);
#endif
// z stepper is only enabled while moving
z_disable();
}
else
{
psu_timeout = 0;
// After having finished, dda_start() will set the timer.
setTimer(dda->c >> 8);
}
//Turn off step outputs, hopefully they've been on long enough by now to register with the drivers,
//if not, too bad. Or insert a (very!) small delay here, or fire up a spare timer or something.
//we also hope that we don't step before the drivers register the low- limit maximum speed if you
//think this is a problem.
unstep();
Rewrite:
#ifdef ACCELERATION_RAMPING
//If there are no steps left or an endstop stop happened, we have finished.
if((move_state.x_steps == 0 && move_state.y_steps == 0 && move_state.z_steps == 0 &&
move_state.e_steps == 0) || (move_state.endstop_stop && dda->n == 0))
{
dda->live = 0;
dda->done = 1;
#ifdef LOOKAHEAD
//If look-ahead was using this move, it could have missed our activation.
//Make sure the ids do not match.
dda->id--;
#endif
#ifdef DC_EXTRUDER
heater_set(DC_EXTRUDER, 0);
#endif
//Z stepper is only enabled while moving
z_disable();
}
else
{
psu_timeout = 0;
// After having finished, dda_start() will set the timer.
setTimer(dda->c >> 8);
}
//Turn off step outputs, hopefully they've been on long enough by now to register with the drivers,
//if not, too bad. Or insert a (very!) small delay here, or fire up a spare timer or something.
//we also hope that we don't step before the drivers register the low- limit maximum speed if you
//think this is a problem.
unstep();
#else
//If there are no steps left or an endstop stop happened, we have finished.
if((move_state.x_steps == 0 && move_state.y_steps == 0 && move_state.z_steps == 0 &&
move_state.e_steps == 0))
{
dda->live = 0;
dda->done = 1;
#ifdef LOOKAHEAD
//If look-ahead was using this move, it could have missed our activation.
//Make sure the ids do not match.
dda->id--;
#endif
#ifdef DC_EXTRUDER
heater_set(DC_EXTRUDER, 0);
#endif
//Z stepper is only enabled while moving
z_disable();
}
else
{
psu_timeout = 0;
// After having finished, dda_start() will set the timer.
setTimer(dda->c >> 8);
}
//Turn off step outputs, hopefully they've been on long enough by now to register with the drivers,
//if not, too bad. Or insert a (very!) small delay here, or fire up a spare timer or something.
//we also hope that we don't step before the drivers register the low- limit maximum speed if you
//think this is a problem.
unstep();
#endif
Second piece of advice, just because brackets are optional, don't exclude them. Sure the compiled code may be identical but human reading of code is more important than the compiler.
Perfect example:
If (foo == bar)
Baz();
Wibble();
This was the root of a rather nasty security bug found last month.
Wibble() was added later and indented. Coder assumed it would only be executed if foo == bar.
Coder was wrong.
Don't skimp the braces, parentheses, squigglys, square brackets or whatever you call them.
Your code should look like white-noise. It's good for maintainence.
Second piece of advice, just because brackets are optional, don't exclude them. Sure the compiled code may be identical but human reading of code is more important than the compiler.
Red
I never intentionally remove optional brackets and I always add the optional brackets that have been excluded.
Jazzed
Not saying I am right, but from my perspective, at least the conditions are compared to the defines. So would you have left the condition as it was?
EDIT: One thing I know for sure, dda.c looks a whole lot better now, then it did this morning
// BLAH_JUNK is defined per platform, it is not a syntax error
if(
(move_state.x_steps == 0 && move_state.y_steps == 0 && move_state.z_steps == 0 && move_state.e_steps == 0)
BLAH_JUNK
)
{
// brace body
}
Option 5.c
Slightly more agreeable, but still ugly.
// If there are no steps left or an endstop stop happened, we have finished.
#ifdef ACCELERATION_RAMPING
if ((move_state.x_steps == 0 && move_state.y_steps == 0 && move_state.z_steps == 0 && move_state.e_steps == 0) ||
(move_state.endstop_stop && dda->n == 0))
#else
if ((move_state.x_steps == 0 && move_state.y_steps == 0 && move_state.z_steps == 0 && move_state.e_steps == 0)
#endif
{
// brace body
}
Now that I've spent an excessive amount of time writing an answer, I probably won't spend any more time on another answer ;-)
One of the biggest problems with starting out in C and C++ is looking at the junk other people provide and deciding whether to do a 'stylistic' revision or to just leave the code in the original author's style.
When I edit English, I leave the author's style alone. But I find myself more tempted to do a wholesale revision of style in computer code.
Just consider the fact that any changes you do will imply that you are willing to explain to others why these are necessary and why they are an improvement.
I am also a bit worried about how I should go about attributing code to the original author when it obviously looks so different from the original.
I also see that Jazzed has mentioned that C++ offers features to clean up the code. I am finally accepting that there is a need for me to learn C++ to get better understanding of what people have written. I just wanted to learn C, but in today's world --- that seems impossible.
Arduino code is very C like, but it is really C++. SimpleIDE manages the compile with C++, even if the user writes faithfully in C. So at some point (when you get stuck), you might be looking at C++ features and wondering what they mean. I guess I have to learn C++ regardless of my limited ambitions.
There are several types of revision and without going into detail about other revisions, I will tell you what I am doing. With the exception of the particular code that Jazzed and I were discussing, I am leaving the authors original code and comments in tact, however the appearance of dda.c has been highly changed. The code indentations were all over the place and in many places, there were spaces instead of tabs, which indicates a lot of cut and paste from different sources. When pasting code, identations get all out of whack and makes it much more difficult to easily read. Additionally, the comments were not laid out that well, in a nice clean fashion. In addition to those issues, the original author left out optional {}.
Basically, I went through dda.c and aligned identations, replaced indentation spaces with tabs, provided better layout for the comments, inserted missing optional brackets, corrected misspellings in the comments, etc.... The code is the essentially the same exact code as the author intended, it just looks better and is easier to read and follow.
The code indentations were all over the place and in many places, there were spaces instead of tabs, which indicates a lot of cut and paste from different sources.
That's ironic.
The biggest reason indentation gets butchered is because of people using tabs. By always using spaces, indentation gets preserved because what you see is what you get. The only time a tab should be used in my opinion is in a make file where it is required.
Still, the rule of original code dominates such things in a collaborative environment. I've seen people fired for re-formatting a file to fix white space on a commit.
The biggest reason indentation gets butchered is because of people using tabs. By always using spaces, indentation gets preserved because what you see is what you get.
Microsoft Visual Studios does not have the same opinion as you, and I am sure their programmers are VERY highly qualified.
EDIT: Another reason indentation gets butchered, is because many editors cannot properly read tabs.
I use Gedit for editing code, just because Forth on the Propeller gets into a tailspin with TABS. The Forth parse is written to ignore SPACE, but has no idea what to do with a TAB in text. (This is ANSI Standard Forth)
Gedit wisely provides the ability to designate your choice of TABS being encoded as \t or as any specified number of spaces. I generate all my TABS as a designated 5 spaces. And I leave on a feature that shows whether the white spaces info is really a TAB or a SPACE or a LF or a CR.
The advantage with using spaces is that when another person opens the .txt file with different tab settings in their display, you still get exactly what was intended to be displayed. Of course, it doesn't surprise me that Microsoft Visual Studios doesn't agree. But that doesn't make Microsoft the highest authority on computation.
There is a UNIX world out there that seems to be more sane and more logical. Be wary of the fact that Microsoft uses UTF-16 while the Unix/Linux/Apple use UTF-8. That may be the origin of the formating mess you refer to.
++++++++++++++
Aside from all the debate about white space....
I am having to clean up the Tonokip_with_serial.zip just to get through a proper debugging.
There is no shortage of sloppy C coders that generate something that works or may work, yet is difficult to read and learn from.
Furthermore, when Jazzed and Martin H helped me with a port over from Arduino to the Propeller, I was handed code that compiled fully... but had a few oddball loose ends. Some of these were created by the porting (which I do appreciate).
In sum, it certainly helps to clean and adequately include intelligent comments. But the skill of good communicative writing of code is not achieved right out of the gate; it may take years to get to the point where it isn't pedantic or cryptic or both.
I personally prefer tabs to maintain indentation, however I am not objective to spaces, as long as proper indentation is maintained. When indentation is not maintained, it makes it very difficult to follow the code, especially if a programmer excludes optional brackets.
dda.c was a mess, with approximately 60% of the document indented with tabs, and the other 40% indented with spaces. And talk about proper indentation, there wasn't any.
"The rule of original code dominates such things in a collaborative environment."
So, as long as all indents are tabs, I don't mind.
The thing that drives me crazy is when someone has an 8 space tabstop set and they use a 4 space indent. I'd rather not contribute at all to code like that.
I personally prefer tabs to maintain indentation, however I am not objective to spaces, as long as proper indentation is maintained. When indentation is not maintained, it makes it very difficult to follow the code, especially if a programmer excludes optional brackets.
dda.c was a mess, with approximately 60% of the document indented with tabs, and the other 40% indented with spaces. And talk about proper indentation, there wasn't any.
Yes, people get into a project where they should use TAB and INDENT properly; but at some point in driving to get the darned thing done, they just start hitting the space bar until everything looks okay.
Automatic INDENT can mystify and annoy the best of us.
But Microsoft has a long sordid history of taking a perfectly good standard and adding in a bunch of quirks so that they can claim ownership. MS doesn't want anything generic and universal in their OS if they can lock in users through fear. And then they foist on to the public that their way is the the absolute best. Remarkably, there is a huge crowd out their that is actually afraid of using Linux because this intimidation has succeeded.
Rule number one: Don't change anything unless you have to. When hacking on somebody else's code never "fix" their formatting style or rearrange their code or comments in anyway more than is required to make the code do what you want.
Rule number two: When adding code or changes to somebody else project use the same formatting and comment style that they have used in the original.
Why?
Firstly because if you only make the minimum changes it's much easier to do a diff between what you started with and what you have created. When your code fails, as it will, it's much easier to track down where the problems might be. Traceability is much easier.
Secondly because, possibly, may be, you can give your changes back to the original author and make his project better. In which case you should make it easy for him to see what you have changed and why. I.e. not buried in a billion useless formatting changes. And he will not throw up on seeing your own personal favorite formatting style.
Thirdly because, assuming your changes are adopted by the "up stream" project, it's much easier to you to adopt their future improvements back into your code.
It's a win all around not to be an Smile about formatting.
Rule number one: Don't change anything unless you have to. When hacking on somebody else's code never "fix" their formatting style or rearrange their code or comments in anyway more than is required to make the code do what you want.
Rule number two: When adding code to somebody else project use the same formatting and comment style that they have used in the original.
Why?
Firstly because if only make the minimum changes it's much easier to do a diff between what you started with and what you have created. When your code fails, as it will, it's much easier to track down where the problems might be.
Secondly because, possibly, may be, you can give your changes back to the original author and make his project better. In which case you should make it easy for him to see what you have changed and why. I.e. not buried in a billion useless formatting changes. And he will not throw up on seeing your own personal favorite formatting style.
@Heater
You advice is remarkably similar to what I follow in editing English text for publication. And over the past 20 years, I have had a near perfect track record of successful publication in helping various Taiwanese resubmit for publication after their English has been initially rejected.
If you wander into trying to migrate their style to your own style, all hell breaks loose.
You did add in the formality of creating a DIFF file, which is a list of all the changes that have taken place in the most recent round of editing. I actually wish OBEX required DIFF files with updates, but it is too late for that.
Still it is nice to know that missing parenthesis and missing curly brackets might be the source of hard to read code. I can always do a markup for just my personal comprehension. I also added empty lines to my clean up of Tonokip_with_serial.zip (which is pending) as the blocks of information seem to run all together.
@idbruce,
I don't understand what you are saying, I was not suggesting any kind of "gold standard". Far from it. I was suggesting it is wise to use the "standard" of whatever project you are hacking on.
Of course if the code you are starting from is a chaotic mess then all bets are off anyway.
But let's imagine a hypothetical case. Say you have spent a year or two perfecting code to do whatever, and published it as open source.
Then I come along and hack it up to work on a different computer or use different devices or add some functionality.
The world would be a better place if I can offer my changes back to you and you add them to your published project. Perhaps other people will find it useful. To that end I should make only the minimal changes and those changes should meet with your approval.
If I totally mess with your formatting, such a trivial thing, it makes that whole process impossibly difficult. We end up with two projects and neither can get the benefit of the work of the other.
@Loopy,
Yes, I can see how that works with "marking up" or editing any kind of prose.
And yes the OBEX needs a DIFF. Actually OBEX should be scrapped and all such code kept in a source code management system like Github. And when you use such a system and collaborate with others you find out why making unnecessary changes to formatting is a really bad idea.
Of course if the code you are starting from is a chaotic mess then all bets are off anyway.
Exactly.
The formatting done to dda.c was for comprehension on my part. In no way do I plan to redistribute this file. The fact of the matter is this, after revising dda.c, I can clearly see that this code is not optimal for the Propeller, however it was nice to see how the author attacked several problems, and how some of the different files are related, as well as their various code sections. I learned a lot yesterday, which was my goal. I will probably reformat some of the other files pertaining to Teacup, just to get a better overall view of the software.
Since revising dda.c, I am now leaning more towards Loopy's goals of having the Propeller house the parser, instead of relying on a PC for parsing. However, since revising dda.c, I am now more convinced, that the software should be studied, keep what is necessary and throw away the rest.
Since revising dda.c, I am now leaning more towards Loopy's goals of having the Propeller house the parser, instead of relying on a PC for parsing. However, since revising dda.c, I am now more convinced, that the software should be studied, keep what is necessary and throw away the rest.
Sounds like you got a lot out of looking through the code. All I got was a headache (just kidding LOL).
Comments
I have decided to go an easier way to get my cnc-mill up and running. For propeller-purists a deadly-sin.
I just bought an arduino clone for $15 a three-axis 3,5A driverboard for $30 loaded the newest GBRL firmware up to the Ardi-clone connected wires and are up and running.
The whole CAD-CAM-toolchain I use is freeware:
Just for testing I made a sphere in google-sketchup cutted the pole of the sphere out. Exported to dae-fileformat. Converted *.dae with MeshLAB to STL. Imported STL-File into ESTLCAM
which generated the gcode. Then used filemode of universal-gcode-sender to send the whole gcode-file down to the arduino-board and milled the part of the sphere out of a massive block of wood.
Wow! never had such a straight forward success with software before. So for me personal I'm having my cnc-mill up and running without a propeller-chip.
P.S.: It was only me who was pointing on DON STARKEYS propeller-CNC-code in the OBEX. I tried it and had problems to get it running.
I will still do smaller projects with the propeller-chip but not a CNC-interpreter.
best regards
Stefan
Hmmm. I got you mixed up with Don Starkey. He provided the CNC code; you revived interest in it.
===========================
I certainly realize that the combination of existing boards and mature code out there for the AVRs make it easiest to just open your checkbook and buy all the items that work.
In my own case, I am wanting to learn more about the Propeller. So, the availability of Adruino code to port to the Propeller seemed a logical way to get up to speed.
Tonokip_with_Serial seems to have been provided in a state where the code was being debugged and incomplete. It is not a very good place to seek out a working port. But I keep fiddling with it to sharpen my skills. Also, the serial port configuration seems to not be right.
I started to port Teacup, but ran into code that provides selection for a wide variety of Arduino chips. Also it appears to be for OpenAVR, not really Arduino specific. So that created quite a few additional topics to think through... even though that code may actually work properly and well on an Arduino.
And then GRBL, for non-3D printer CNC projects seems to be the clearest of these 3 choices for porting. But it will not provide the additions need for a filament printer head and bed heater. So in spite of it being the most likable code to begin with, it leads only so far.
What am I trying to say? I guess I am just explaining why I have so little to show at this point. Someone else, with more experience with C and C++ may have gotten this all resolved by now. But I am not getting any volunteers to lead the way.
I keep plodding on as I am enjoying finally learning to use C and C++ on the Propeller. There is a faster track if you don't want to learn what the actual code is all about.
+++++++++++++
In all honesty, a Propeller 2 or Propeller 1+ chip might really make this project move ahead with a much more feasible, full-featured 3D printer g-code interpreter. Space has been an issue all along.
Okay, I came up with another one for the C gurus...
While editing dda.c, I came across the following assignment:
dda->x_step_interval = dda->y_step_interval = \ dda->z_step_interval = dda->e_step_interval = 0xFFFFFFFF;It appears to be a bit off to me, more particularly "= \". What do the gurus say?
EDIT: Now I have found it used another time. I assume it must be legal.
It seems that if the G1 has run its complete series of x, y, z, and e steps; the code is support to consider that one G1 sequence complete. Acceleration and deceleration are ramped up and brought down with each and every G1 call.
In other words, this just appears to clean up the the G1 and make ready for the next G1 run if Acceleration Ramping is implemented.
I have been studying the GRBL CNC code to better understand the acceleration ramping and Teacup seems to not explain all the details.
+++++++++++++++++++++++++++
I find is very odd and confusing that the thread title can be hijacked by any poster. I didn't know that. I have returned it to the original.
LOL. I've seen worse.
Now you know why software costs more than hardware :-)
Original:
// If there are no steps left or an endstop stop happened, we have finished. if ((move_state.x_steps == 0 && move_state.y_steps == 0 && move_state.z_steps == 0 && move_state.e_steps == 0) #ifdef ACCELERATION_RAMPING || (move_state.endstop_stop && dda->n == 0) #endif ) { dda->live = 0; dda->done = 1; #ifdef LOOKAHEAD // If look-ahead was using this move, it could have missed our activation: // make sure the ids do not match. dda->id--; #endif #ifdef DC_EXTRUDER heater_set(DC_EXTRUDER, 0); #endif // z stepper is only enabled while moving z_disable(); } else { psu_timeout = 0; // After having finished, dda_start() will set the timer. setTimer(dda->c >> 8); } //Turn off step outputs, hopefully they've been on long enough by now to register with the drivers, //if not, too bad. Or insert a (very!) small delay here, or fire up a spare timer or something. //we also hope that we don't step before the drivers register the low- limit maximum speed if you //think this is a problem. unstep();Rewrite:
#ifdef ACCELERATION_RAMPING //If there are no steps left or an endstop stop happened, we have finished. if((move_state.x_steps == 0 && move_state.y_steps == 0 && move_state.z_steps == 0 && move_state.e_steps == 0) || (move_state.endstop_stop && dda->n == 0)) { dda->live = 0; dda->done = 1; #ifdef LOOKAHEAD //If look-ahead was using this move, it could have missed our activation. //Make sure the ids do not match. dda->id--; #endif #ifdef DC_EXTRUDER heater_set(DC_EXTRUDER, 0); #endif //Z stepper is only enabled while moving z_disable(); } else { psu_timeout = 0; // After having finished, dda_start() will set the timer. setTimer(dda->c >> 8); } //Turn off step outputs, hopefully they've been on long enough by now to register with the drivers, //if not, too bad. Or insert a (very!) small delay here, or fire up a spare timer or something. //we also hope that we don't step before the drivers register the low- limit maximum speed if you //think this is a problem. unstep(); #else //If there are no steps left or an endstop stop happened, we have finished. if((move_state.x_steps == 0 && move_state.y_steps == 0 && move_state.z_steps == 0 && move_state.e_steps == 0)) { dda->live = 0; dda->done = 1; #ifdef LOOKAHEAD //If look-ahead was using this move, it could have missed our activation. //Make sure the ids do not match. dda->id--; #endif #ifdef DC_EXTRUDER heater_set(DC_EXTRUDER, 0); #endif //Z stepper is only enabled while moving z_disable(); } else { psu_timeout = 0; // After having finished, dda_start() will set the timer. setTimer(dda->c >> 8); } //Turn off step outputs, hopefully they've been on long enough by now to register with the drivers, //if not, too bad. Or insert a (very!) small delay here, or fire up a spare timer or something. //we also hope that we don't step before the drivers register the low- limit maximum speed if you //think this is a problem. unstep(); #endifEDIT: I now see some parenthesis can be removed
The problem is that now there are two near identical blocks of code to maintain. Which one will the next contributor change?
This is where C++ shines for those with enough knowledge and practice to use it effectively.
If the if() {} braced body was a function, it would be easier to swallow.
Second piece of advice, just because brackets are optional, don't exclude them. Sure the compiled code may be identical but human reading of code is more important than the compiler.
Perfect example:
If (foo == bar)
Baz();
Wibble();
This was the root of a rather nasty security bug found last month.
Wibble() was added later and indented. Coder assumed it would only be executed if foo == bar.
Coder was wrong.
Don't skimp the braces, parentheses, squigglys, square brackets or whatever you call them.
Your code should look like white-noise. It's good for maintainence.
Red
I never intentionally remove optional brackets and I always add the optional brackets that have been excluded.
Jazzed
Not saying I am right, but from my perspective, at least the conditions are compared to the defines. So would you have left the condition as it was?
EDIT: One thing I know for sure, dda.c looks a whole lot better now, then it did this morning
I would prefer to do something more reasonable depending on the factors in play.
Useful alternatives in order of desirability:
1. Leave as is if and only if it's expected to be committed back to someone else's code base.
2. Make it a class and extend it correctly if it's C++. (localized redesign)
3. Use a struct parameter pointer that defines the character of how the device drivers are used. (changes many things)
4. Use function pointers which is over-kill, and often hard to follow in a debugger. (changes many things)
5. Use a cleaner variation of the hack. (localized re-hack)
Instead of this:
if(
(move_state.x_steps == 0 && move_state.y_steps == 0 && move_state.z_steps == 0 && move_state.e_steps == 0)
#ifdef BLAH
|| (move_state.endstop_stop && dda->n == 0)
#endif
)
Option 5.a
Assuming BLAH is the #define that is used everywhere ....
#ifdef BLAH
#define USEBLAH 1
#else
#define USEBLAH 0
#endif
if(
(move_state.x_steps == 0 && move_state.y_steps == 0 && move_state.z_steps == 0 && move_state.e_steps == 0)
|| (USEBLAH && (move_state.endstop_stop && dda->n == 0)) )
{
// brace body
}
Option 5.b
Assuming BLAH is the #define that is used everywhere .... and pressed for time or code space.
#ifdef BLAH
#define BLAH_JUNK "|| (move_state.endstop_stop && dda->n == 0)"
#else
#define BLAH_JUNK ""
#endif
// BLAH_JUNK is defined per platform, it is not a syntax error
if(
(move_state.x_steps == 0 && move_state.y_steps == 0 && move_state.z_steps == 0 && move_state.e_steps == 0)
BLAH_JUNK
)
{
// brace body
}
Option 5.c
Slightly more agreeable, but still ugly.
// If there are no steps left or an endstop stop happened, we have finished.
#ifdef ACCELERATION_RAMPING
if ((move_state.x_steps == 0 && move_state.y_steps == 0 && move_state.z_steps == 0 && move_state.e_steps == 0) ||
(move_state.endstop_stop && dda->n == 0))
#else
if ((move_state.x_steps == 0 && move_state.y_steps == 0 && move_state.z_steps == 0 && move_state.e_steps == 0)
#endif
{
// brace body
}
Now that I've spent an excessive amount of time writing an answer, I probably won't spend any more time on another answer ;-)
When I edit English, I leave the author's style alone. But I find myself more tempted to do a wholesale revision of style in computer code.
Just consider the fact that any changes you do will imply that you are willing to explain to others why these are necessary and why they are an improvement.
I am also a bit worried about how I should go about attributing code to the original author when it obviously looks so different from the original.
I also see that Jazzed has mentioned that C++ offers features to clean up the code. I am finally accepting that there is a need for me to learn C++ to get better understanding of what people have written. I just wanted to learn C, but in today's world --- that seems impossible.
Arduino code is very C like, but it is really C++. SimpleIDE manages the compile with C++, even if the user writes faithfully in C. So at some point (when you get stuck), you might be looking at C++ features and wondering what they mean. I guess I have to learn C++ regardless of my limited ambitions.
There are several types of revision and without going into detail about other revisions, I will tell you what I am doing. With the exception of the particular code that Jazzed and I were discussing, I am leaving the authors original code and comments in tact, however the appearance of dda.c has been highly changed. The code indentations were all over the place and in many places, there were spaces instead of tabs, which indicates a lot of cut and paste from different sources. When pasting code, identations get all out of whack and makes it much more difficult to easily read. Additionally, the comments were not laid out that well, in a nice clean fashion. In addition to those issues, the original author left out optional {}.
Basically, I went through dda.c and aligned identations, replaced indentation spaces with tabs, provided better layout for the comments, inserted missing optional brackets, corrected misspellings in the comments, etc.... The code is the essentially the same exact code as the author intended, it just looks better and is easier to read and follow.
That's ironic.
The biggest reason indentation gets butchered is because of people using tabs. By always using spaces, indentation gets preserved because what you see is what you get. The only time a tab should be used in my opinion is in a make file where it is required.
Still, the rule of original code dominates such things in a collaborative environment. I've seen people fired for re-formatting a file to fix white space on a commit.
Microsoft Visual Studios does not have the same opinion as you, and I am sure their programmers are VERY highly qualified.
EDIT: Another reason indentation gets butchered, is because many editors cannot properly read tabs.
Gedit wisely provides the ability to designate your choice of TABS being encoded as \t or as any specified number of spaces. I generate all my TABS as a designated 5 spaces. And I leave on a feature that shows whether the white spaces info is really a TAB or a SPACE or a LF or a CR.
The advantage with using spaces is that when another person opens the .txt file with different tab settings in their display, you still get exactly what was intended to be displayed. Of course, it doesn't surprise me that Microsoft Visual Studios doesn't agree. But that doesn't make Microsoft the highest authority on computation.
There is a UNIX world out there that seems to be more sane and more logical. Be wary of the fact that Microsoft uses UTF-16 while the Unix/Linux/Apple use UTF-8. That may be the origin of the formating mess you refer to.
++++++++++++++
Aside from all the debate about white space....
I am having to clean up the Tonokip_with_serial.zip just to get through a proper debugging.
There is no shortage of sloppy C coders that generate something that works or may work, yet is difficult to read and learn from.
Furthermore, when Jazzed and Martin H helped me with a port over from Arduino to the Propeller, I was handed code that compiled fully... but had a few oddball loose ends. Some of these were created by the porting (which I do appreciate).
In sum, it certainly helps to clean and adequately include intelligent comments. But the skill of good communicative writing of code is not achieved right out of the gate; it may take years to get to the point where it isn't pedantic or cryptic or both.
I personally prefer tabs to maintain indentation, however I am not objective to spaces, as long as proper indentation is maintained. When indentation is not maintained, it makes it very difficult to follow the code, especially if a programmer excludes optional brackets.
dda.c was a mess, with approximately 60% of the document indented with tabs, and the other 40% indented with spaces. And talk about proper indentation, there wasn't any.
LOL.
There is always some silly argument in programming.
I come in peace.
Ya. Me too.
"The rule of original code dominates such things in a collaborative environment."
So, as long as all indents are tabs, I don't mind.
The thing that drives me crazy is when someone has an 8 space tabstop set and they use a 4 space indent. I'd rather not contribute at all to code like that.
Yes, people get into a project where they should use TAB and INDENT properly; but at some point in driving to get the darned thing done, they just start hitting the space bar until everything looks okay.
Automatic INDENT can mystify and annoy the best of us.
But Microsoft has a long sordid history of taking a perfectly good standard and adding in a bunch of quirks so that they can claim ownership. MS doesn't want anything generic and universal in their OS if they can lock in users through fear. And then they foist on to the public that their way is the the absolute best. Remarkably, there is a huge crowd out their that is actually afraid of using Linux because this intimidation has succeeded.
Rule number two: When adding code or changes to somebody else project use the same formatting and comment style that they have used in the original.
Why?
Firstly because if you only make the minimum changes it's much easier to do a diff between what you started with and what you have created. When your code fails, as it will, it's much easier to track down where the problems might be. Traceability is much easier.
Secondly because, possibly, may be, you can give your changes back to the original author and make his project better. In which case you should make it easy for him to see what you have changed and why. I.e. not buried in a billion useless formatting changes. And he will not throw up on seeing your own personal favorite formatting style.
Thirdly because, assuming your changes are adopted by the "up stream" project, it's much easier to you to adopt their future improvements back into your code.
It's a win all around not to be an Smile about formatting.
Here is a nice place to express that view (http://forums.parallax.com/showthread.php/137491-Gold-Standard-Objects-Up-for-Commission)
You advice is remarkably similar to what I follow in editing English text for publication. And over the past 20 years, I have had a near perfect track record of successful publication in helping various Taiwanese resubmit for publication after their English has been initially rejected.
If you wander into trying to migrate their style to your own style, all hell breaks loose.
You did add in the formality of creating a DIFF file, which is a list of all the changes that have taken place in the most recent round of editing. I actually wish OBEX required DIFF files with updates, but it is too late for that.
Still it is nice to know that missing parenthesis and missing curly brackets might be the source of hard to read code. I can always do a markup for just my personal comprehension. I also added empty lines to my clean up of Tonokip_with_serial.zip (which is pending) as the blocks of information seem to run all together.
I don't understand what you are saying, I was not suggesting any kind of "gold standard". Far from it. I was suggesting it is wise to use the "standard" of whatever project you are hacking on.
Of course if the code you are starting from is a chaotic mess then all bets are off anyway.
But let's imagine a hypothetical case. Say you have spent a year or two perfecting code to do whatever, and published it as open source.
Then I come along and hack it up to work on a different computer or use different devices or add some functionality.
The world would be a better place if I can offer my changes back to you and you add them to your published project. Perhaps other people will find it useful. To that end I should make only the minimal changes and those changes should meet with your approval.
If I totally mess with your formatting, such a trivial thing, it makes that whole process impossibly difficult. We end up with two projects and neither can get the benefit of the work of the other.
@Loopy,
Yes, I can see how that works with "marking up" or editing any kind of prose.
And yes the OBEX needs a DIFF. Actually OBEX should be scrapped and all such code kept in a source code management system like Github. And when you use such a system and collaborate with others you find out why making unnecessary changes to formatting is a really bad idea.
Exactly.
The formatting done to dda.c was for comprehension on my part. In no way do I plan to redistribute this file. The fact of the matter is this, after revising dda.c, I can clearly see that this code is not optimal for the Propeller, however it was nice to see how the author attacked several problems, and how some of the different files are related, as well as their various code sections. I learned a lot yesterday, which was my goal. I will probably reformat some of the other files pertaining to Teacup, just to get a better overall view of the software.
Since revising dda.c, I am now leaning more towards Loopy's goals of having the Propeller house the parser, instead of relying on a PC for parsing. However, since revising dda.c, I am now more convinced, that the software should be studied, keep what is necessary and throw away the rest.
Sounds like you got a lot out of looking through the code. All I got was a headache (just kidding LOL).