pinio c + h are not working properly, as well as various calls from other places. So I have decided to eliminate PropellerizedTeacup.h, and rewrite pinio c + h using simpletools library.
As I see it, the previous upload is a good place to actually start the port. However, there are a couple of things that just bother me a little, because they have no value for this port. So before I dig in deep for the next round, I am going to go through all the files one by one, and remove those little annoyances. When I get finished with that process, I will upload the result of that effort, and that will be the starting point of my next round.
As some of you may know, by reading this thread or from downloading the project, there are several files missing, which prevent the possibility of being functional. During the next round, I will be attempting to replace some of those missing files.
Just letting you know to expect a new upload soon.
As I see it, the previous upload is a good place to actually start the port. However, there are a couple of things that just bother me a little, because they have no value for this port. So before I dig in deep for the next round, I am going to go through all the files one by one, and remove those little annoyances. When I get finished with that process, I will upload the result of that effort, and that will be the starting point of my next round.
As some of you may know, by reading this thread or from downloading the project, there are several files missing, which prevent the possibility of being functional. During the next round, I will be attempting to replace some of those missing files.
Just letting you know to expect a new upload soon.
The annoyances that I was looking for are actually in the missing files. However, as it pertains to the last upload, I made two changes, which were as follows:
and in dda_queue.c, at the top of the file, remove the following line completely, because delay.h will not be added to this project.
//2/25/2015//Bruce//#include "delay.h"
Additionally for general knowledge and reference to previous discussion, and considering that several people are now downloading this project, there is something that needs to be reestablished. The previous uploaded project will build without error, however there is quite an array of warnings. Most, if not all of these warnings, are directly related to sersendf.c and stdarg.h. For more information pertaining to these warnings, please refer to POSTS 46~49. If anyone can come up with a better solution to resolve these warnings, that would be a good thing.
Bruce, it seems like you would want to minimize the changes to most of the files. So why not define SET_OUT and SET_OUTPUT macros that do the right thing on the Prop instead of using new macros? Also, you could make delay.h and empty file instead of commenting out the include in every file that includes it.
Bruce, it seems like you would want to minimize the changes to most of the files. So why not define SET_OUT and SET_OUTPUT macros that do the right thing on the Prop instead of using new macros?
Basically they are same thing as the originals, but with the names changed, for clarity sake, and instead of using Arduino register names in the define, I am now using Propeller register names. I don't know what happened, but while testing your macros several days ago, a pin that should have been an output, was registered as an input. Upon retesting your macros last night, everything was fine, but by that time, I had already altered all the files. The macros that I have now, test out just a fine, but I did not do any extensive testing. I believe they should be correct, with the exception that some are in a do-while. With the exception of the do-while(s), the code was basically copied from the simpletools library. Now perhaps the do-while(s) could be eliminated, because I am not sure what purpose they hold, if any, but the defines are functioning, as verified by testing.
Also, you could make delay.h and empty file instead of commenting out the include in every file that includes it.
Instead of commenting out the include, I will be deleting all references to delay.h, within all files, because it is not needed. Of course there was only one comment in the uploaded project, but I have started working on the missing files, and those files will not have any reference to delay.h.
In case you or anyone else is interested, I have attached the test project that I put together for the macros.
Bruce, you may want to try the simulator mode. Look at the READ.sim file to see how to build and run it. I was able to get it running under linux in less than a half hour. I copied config.teensy.h to config.h and ThermistorTable.single.h to ThermistorTable.h. I also had to add a #define for pgm_read_dword in simulator.h. Also, there wasn't a clock_gettime() function in the library, so I had to add that to timer_ext.c. I used the following code.
I was able to run one of the sample g-code files by typing "./sim -v -g -p -t=0 testcases/triangle.gcode".
The nice thing about starting with the simulator mode is that it removes most of the AVR dependencies. Once you get the simulator mode running you can then replace the various driver files with Prop-specific code.
BTW, I didn't try running this on the Prop, but it will require using the XMMC model with external memory. If you have a board with an SD card you can use that for external memory, though it is slow. Flash memory is the best choice for running XMMC programs.
When I was younger and first started programming in C/C++, I tried the "makefile" thing on several instances and all it ever gave me was grief So I gave up, without ever learning how to do it properly. I suppose, I should really learn what makefile is all about, and at least manage one successful build
Back to the subject at hand....
I will look into it, but it is really too late at this point to add simulator support for the project that I have well underway. Every reference to the simulator has been eliminated
However, now that I have a fairly well round knowledge of the firmware, once I learn how to do the "makefile" thing, I may go over the project one final time to reestablish support for the simulator.
As always, I appreciate your input on the subject, and I will eventually look into and achieve a successful makefile build
As mentioned elsewhere in the forum, I will be using a micro SD card to supply my GCODE to the firmware, whereas the main function within forger, currently relies on a serial connection, with the following code:
if ((serial_rxchars() != 0) && (queue_full() == 0))
{
uint8_t c = serial_popchar();
gcode_parse_char(c);
}
For those folks who want a serial feed, you will have to design a character feed mechanism. And for those folks who want to read characters from an SD card one by one, in a different project, I am uploading a sample project which contains the code below and this code is very similar to that which I will be using in forger.
EDIT: Or perhaps I should say temporarily. Eventually, I will probably provide several different sources for chacheter feeds, but for now, I think I will just stick with SD, at least until I get the firmware and 3D printer fully functional.
EDIT: Well I changed my mind, I am adding both SD and serial support.
#include "simpletools.h"
// SD card pins on Propeller BOE
#define DO 22
#define CLK 23
#define DI 24
#define CS 25
int main(void)
{
// Mount SD card
sd_mount(DO, CLK, DI, CS);
FILE * pFile;
int c;
pFile = fopen("minimum.txt","r");
if(pFile == NULL)
{
putStr("Error opening file");
}
else
{
do
{
c = getc(pFile);
putChar(c);
}
while(c != EOF);
fclose (pFile);
}
return 0;
}
I have a new upload for you folks. This upload includes several changes and several new files have been added.
A global full duplex serial object has been added to the project with the addition of sergcode.c and sergcode.h
Provided support for both SD card and serial port parsing
The main application file has been altered quite a bit
Extended config.h file options
A little bit of housekeeping and cleanup
Renamed the thermistor tables and added them as thermistor.h and thermistors.h, for single or double thermistor support
Please note that the "sergcode" pointer utilized with the parse_serial(void) function is a global full duplex serial pointer. This pointer can be used whereever the sergcode header file is included, e.g.: #include "sergcode.h", and this pointer will most likely be used to replace most of the remaining serial interface references, but I still need to take a good look at that. My machine will have a serial LCD display, and I still need to determine what messages need to go to that display.
I suppose at this point, a person could alter the config.h file, and uncomment the main function within forger.c, and give it a test go to see what happens, but personally I am not brave enough just yet Perhaps that is best anyhow, because without the proper serial output, what's the point?
I am providing this upload, so that others may see how the project is progressing, and what direction it is heading. Additionally, I am providing it so that others may suggest ideas and/or take it in a direction of their own choosing.
Oh..... The source of the GCODE must be chosen in config.h, and set to either SD card or serial input.
It took me a while, but I now have an intended course of action. The determining factor for changing my course was the purchase of a 12 bit ADC instead of a 10 bit ADC. Most of the support source code available is designed around the use of a 10 bit ADC, and since mine is 12 bit, I could either try to decipher the mess or I could start with a clean slate. Considering that heater.c, heater.h, temp.c, temp.h, analog.c, and analog.h, would be a complete headache to port, I am simply going to abandoned all those files and start fresh.
Now that I have support code for the ADS1015 12 bit/4 channel ADC that I purchased, I am going to start building my new files around that code, in addition to other source code that I found for obtaining thermistor temperatures. The source code for obtaining thermistor temperatures can be found here: https://learn.adafruit.com/thermistor/using-a-thermistor.
If you visited the link provided, you will see that I am no longer going to be relying on thermistor tables, instead I will be generating an unknown amount of samples for both the heated bed and the extruder, and averaging the temperatures on the fly. Most likely I will have external current temperature variables for both the heated bed and the extruder, and gcode_process.c + h, will most likely have external target temperature variables for both the heated bed and the extruder. These four variables (bed_target_temp, extruder_target_temp, bed_current_temp, extruder_current_temp) will be monitored by the ADS1015 library, and this library will further control the pwm going to both heaters and additionally the extruder fan. By having the ADC source code control all of the heating and temperature related issues, I believe that will greatly simplify this port.
Well here it is....... The build without any warnings
Many changes have occured since I last updated the archive in this thread. Instead of me describing all of the changes, I invite you to take a good look at the source. Sorry, but I am all typed out for now.
However I will say that it still needs some polishing to get it to where I want it for the next step.
EDIT: I suppose I should tell you that at this point, there are two serial objects. One is provided by SimpleTools and there is another initiated sergcode.c.
Any particular reason why you did not fork the original github repo and work on you own branch. Then we could easily see what you have changed. There is no way anyone is going to read all that to see what is changed/added or fixed/broken.
Any particular reason why you did not fork the original github repo and work on you own branch.
Two reasons really...
1) I can't access github with IE and I particularly don't like using Chrome, unless absolutely necessary.
2) And I never tried it before. Not really interested in learning all the ins and outs of github
I do not believe you cannot use github from IE. They would be dead in the water if that were true.
You don't actually need to use a browser to clone fork and other wise work with repositories on github.
If you are working on any kind of code base bigger than 1 line that will evolve over time, especially if it is derived from another project in git, the git saves you so much headache.
Really, it's worth becoming a bit familiar with. Even if it's a project that will never leave your own computer.
Ah, OK. I did not realize you are living in the past
For the changes I just made, I would spend hours and hours documenting it.
That's a good point. Had you been using git every little change would already be recorded in the commit history.
I avoided source code management systems for years. They were all universally horrible. Like MS Visual Source Safe, Rational/IBM Clear Case, and a bunch of others I forget. Basically more work than they are worth. Only used them when people paid me to:)
git changes all of that. There are only a few git commands you need to know to have it be a God send even on a very small project even if you never share or collaborate with anyone else: clone, add, commit, rm, mv, log, status, diff, branch, merge, checkout, pull, push.
Than again their are GUI interfaces if you are shy of the command line. I have never looked into those much. Apart from github that is.
If for no other reason, Bruce, other folks are a lot more likely to help you if they have easy access to the source code. I can quickly fork your repository, make a change/fix/enhancement/whatever, and then submit a "pull request". Github does this very nicely, and it's much easier than trying to merge code by hand (if, for instance, you and I were both working on the same file).
It can be a lot to learn, but will surely pay off. Commit history is wonderful and, as heater said, can work as basic documentation of all your changes.
I have been avoiding this particular item like a dead and smelly varmint, but I do believe timer.c is the next major piece of the puzzle to resolve. I do believe this file will take some serious thought and study, but it will eventually have to be ported and I think this file is slowing me down. By porting timer.c, I can then add it to the project, along with files timer.h, clock.c, and clock.h.
As mentioned earlier within this thread, I believe that timer.c is the main speed limitation for Teacup, which if I am not mistaken, maxes out at 20MHz. There are actually two timers that run, one for maintenance and another for stepping through the moves.
I was thinking about cog usage last night and that cogs might be running slim, however I believe, in order to make a much smoother and easier port of this firmware, each of these timers should probably have there own cog. By running each of these timers in their own cog, I believe it will pretty much eliminate the use of interrupts throughout the remaining source.
If anyone has input that they would like to share pertaining to the setup of these timers, then I am all ears.
EDIT: Additionally, it appears that all references to a memory barrier seem to pertain to a bug in one of the AVR libraries, so I suppose I could pretty much chuck memory_barrier.h out the window and all references to the file and it's functions. However, I believe I may be treading on thin ice and it may make it more difficult later to track down my own set of bugs but what the heck, it is gone!
memory_barrier.h is all about disabling and enabling interrupts. Let's ignore that problem with the bug they mention.
If you consider that code running in the "background" and code running within an interrupt handler is actually two threads, which might well be running on two cogs on the Prop, then you see that disabling/enabling interrupts is actually a lock mechanism. Rather like claiming and releasing a lock on the Prop.
That of course is all about sharing data between threads without having them both access the same data at the same time and getting corrupt results. It removes data races.
Take that out and you will have a randomly failing program!
As the Prop has no interrupts we don't need to enable/disable them but I believe you will be running background and interrupt handler code on different COGs so you will no doubt need to acquire/release a lock instead to avoid data races.
Yea, that is also my belief, but their code is not going to perform the necessary task on the Propeller, so it is gone. There is no doubt that I will have to reestablish locking mechanisms, but I will cross that bridge, just as soon as I get the timer all straightened out. I will have to do some studying on the use of Propeller locks.
Presumably the code is sprinkled here and there with ATOMIC_START and ATOMIC_END wrapped around critical regions, where shared data is accessed.
Those are exactly the places you will probably want to acquire and release locks if those threads are run from separate COGs.
The correct solution is to keep that file and add #ifdef PROPELLER / #else around what will be used on the Propeller when you get to it. I.e the lock handling code.
As is often stated the aim should be to make as few changes to higher level code as possible. And this is a fine example.
However, I believe there will also be several instances where locks will not be needed. For instance, consider the following code, I believe this can be accomplished without a lock.
void setTimer(uint32_t delay)
{
uint16_t step_start = 0;
// An interrupt would make all our timing calculations invalid,
// so stop that here.
cli();
CLI_SEI_BUG_MEMORY_BARRIER();
// Assume all steps belong to one move. Within one move the delay is
// from one step to the next one, which should be more or less the same
// as from one step interrupt to the next one. The last step interrupt happend
// at OCR1A, so start delay from there.
step_start = OCR1A;
next_step_time = delay;
// Now we know how long we actually want to delay, so set the timer.
if(next_step_time < 65536)
{
// set the comparator directly to the next real step
OCR1A = (next_step_time + step_start) & 0xFFFF;
}
else if(next_step_time < 75536)
{
// Next comparator interrupt would have to trigger another
// interrupt within a short time (possibly within 1 cycle).
// Avoid the impossible by firing the interrupt earlier.
OCR1A = (step_start - 10000) & 0xFFFF;
next_step_time += 10000;
}
else
{
OCR1A = step_start;
}
// Enable this interrupt, but only do it after disabling
// global interrupts (see above). This will cause push any possible
// timer1a interrupt to the far side of the return, protecting the
// stack from recursively clobbering memory.
TIMSK1 |= MASK(OCIE1A);
}
Presumably the code is sprinkled here and there with ATOMIC_START and ATOMIC_END wrapped around critical regions, where shared data is accessed.
Those are exactly the places you will probably want to acquire and release locks if those threads are run from separate COGs.
The correct solution is to keep that file and add #ifdef PROPELLER / #else around what will be used on the Propeller when you get to it. I.e the lock handling code.
I would have to respectfully disagree. I believe that the file should be eliminated and new locks should be etablished where they are required, with Propeller specific functions for such matters.
So in other words, wherever ATOMIC_START and ATOMIC_END are utilized, new locks should be established using the locknew(), lockret(lockid), lockset(lockid), and lockclr(lockid) functions.
Well OK, that can be true. If they are disabling interrupts so as not to disturb timing in some code sequence rather than protect shared data/resource access then you will not need that when running on separate COGs.
I have to respectfully disagree with your respectful disagreement:)
If there are cases of shared resource protection, I have not trawled through the code to check, then you should use the same file and #ifdef some Propeller equivalent code in there. That is the normal and expected practice.
At the end of the day, when you have everything tested and working you should be able to offer your changes back to the original project maintainer and he could merge then into his code base as a new supported platform. If you hack to much unnecessarily he is unlikely to want to do that.
Edit:
So in other words, wherever ATOMIC_START and ATOMIC_END are utilized, new locks should be established using the locknew(), lockret(lockid), lockset(lockid), and lockclr(lockid) functions.
Yes, and that lockxxx code should go into the same file as the the current ATOMIC operations.
Yes, and that lockxxx code should go into the same file as the the current ATOMIC operations.
Yea, that is what I am trying to say, and just eliminate memory_barrier.h completely, as well as ATOMIC_START and ATOMIC_END references, and hopefully it will work instead of causing me a lot of grief.
Comments
LOL Now thats funny.
pinio c + h are not working properly, as well as various calls from other places. So I have decided to eliminate PropellerizedTeacup.h, and rewrite pinio c + h using simpletools library.
EDIT: Sorry deleted the upload due to some errors I found. Be back up soon.
EDIT: Errors fixed.... Here is a new one.
As some of you may know, by reading this thread or from downloading the project, there are several files missing, which prevent the possibility of being functional. During the next round, I will be attempting to replace some of those missing files.
Just letting you know to expect a new upload soon.
The annoyances that I was looking for are actually in the missing files. However, as it pertains to the last upload, I made two changes, which were as follows:
In pinio.h, the following change was made:
FROM:
inline void power_init(void) { #ifdef PS_MOSFET_PIN SET_OUTA(PS_MOSFET_PIN, 0); SET_OUTPUT(PS_MOSFET_PIN); #endif }TO:
inline void power_init(void) { #ifdef PS_MOSFET_PIN SET_OUTA(PS_MOSFET_PIN, 0); SET_DIRA_OUTPUT(PS_MOSFET_PIN); #endif }and in dda_queue.c, at the top of the file, remove the following line completely, because delay.h will not be added to this project.
Additionally for general knowledge and reference to previous discussion, and considering that several people are now downloading this project, there is something that needs to be reestablished. The previous uploaded project will build without error, however there is quite an array of warnings. Most, if not all of these warnings, are directly related to sersendf.c and stdarg.h. For more information pertaining to these warnings, please refer to POSTS 46~49. If anyone can come up with a better solution to resolve these warnings, that would be a good thing.
Basically they are same thing as the originals, but with the names changed, for clarity sake, and instead of using Arduino register names in the define, I am now using Propeller register names. I don't know what happened, but while testing your macros several days ago, a pin that should have been an output, was registered as an input. Upon retesting your macros last night, everything was fine, but by that time, I had already altered all the files. The macros that I have now, test out just a fine, but I did not do any extensive testing. I believe they should be correct, with the exception that some are in a do-while. With the exception of the do-while(s), the code was basically copied from the simpletools library. Now perhaps the do-while(s) could be eliminated, because I am not sure what purpose they hold, if any, but the defines are functioning, as verified by testing.
Instead of commenting out the include, I will be deleting all references to delay.h, within all files, because it is not needed. Of course there was only one comment in the uploaded project, but I have started working on the missing files, and those files will not have any reference to delay.h.
In case you or anyone else is interested, I have attached the test project that I put together for the macros.
int clock_gettime(int clk_id, struct timespec *t) { struct timeval tv; gettimeofday(&tv, 0); t->tv_sec = tv.tv_sec; t->tv_nsec = tv.tv_usec * 1000; return 0; }I was able to run one of the sample g-code files by typing "./sim -v -g -p -t=0 testcases/triangle.gcode".The nice thing about starting with the simulator mode is that it removes most of the AVR dependencies. Once you get the simulator mode running you can then replace the various driver files with Prop-specific code.
BTW, I didn't try running this on the Prop, but it will require using the XMMC model with external memory. If you have a board with an SD card you can use that for external memory, though it is slow. Flash memory is the best choice for running XMMC programs.
When I was younger and first started programming in C/C++, I tried the "makefile" thing on several instances and all it ever gave me was grief
Back to the subject at hand....
I will look into it, but it is really too late at this point to add simulator support for the project that I have well underway. Every reference to the simulator has been eliminated
However, now that I have a fairly well round knowledge of the firmware, once I learn how to do the "makefile" thing, I may go over the project one final time to reestablish support for the simulator.
As always, I appreciate your input on the subject, and I will eventually look into and achieve a successful makefile build
if ((serial_rxchars() != 0) && (queue_full() == 0)) { uint8_t c = serial_popchar(); gcode_parse_char(c); }For those folks who want a serial feed, you will have to design a character feed mechanism. And for those folks who want to read characters from an SD card one by one, in a different project, I am uploading a sample project which contains the code below and this code is very similar to that which I will be using in forger.
EDIT: Or perhaps I should say temporarily. Eventually, I will probably provide several different sources for chacheter feeds, but for now, I think I will just stick with SD, at least until I get the firmware and 3D printer fully functional.
EDIT: Well I changed my mind, I am adding both SD and serial support.
#include "simpletools.h" // SD card pins on Propeller BOE #define DO 22 #define CLK 23 #define DI 24 #define CS 25 int main(void) { // Mount SD card sd_mount(DO, CLK, DI, CS); FILE * pFile; int c; pFile = fopen("minimum.txt","r"); if(pFile == NULL) { putStr("Error opening file"); } else { do { c = getc(pFile); putChar(c); } while(c != EOF); fclose (pFile); } return 0; }I have a new upload for you folks. This upload includes several changes and several new files have been added.
A global full duplex serial object has been added to the project with the addition of sergcode.c and sergcode.h
Provided support for both SD card and serial port parsing
The main application file has been altered quite a bit
Extended config.h file options
A little bit of housekeeping and cleanup
Renamed the thermistor tables and added them as thermistor.h and thermistors.h, for single or double thermistor support
Please note that the "sergcode" pointer utilized with the parse_serial(void) function is a global full duplex serial pointer. This pointer can be used whereever the sergcode header file is included, e.g.: #include "sergcode.h", and this pointer will most likely be used to replace most of the remaining serial interface references, but I still need to take a good look at that. My machine will have a serial LCD display, and I still need to determine what messages need to go to that display.
I suppose at this point, a person could alter the config.h file, and uncomment the main function within forger.c, and give it a test go to see what happens, but personally I am not brave enough just yet
I am providing this upload, so that others may see how the project is progressing, and what direction it is heading. Additionally, I am providing it so that others may suggest ideas and/or take it in a direction of their own choosing.
Oh..... The source of the GCODE must be chosen in config.h, and set to either SD card or serial input.
Now that I have support code for the ADS1015 12 bit/4 channel ADC that I purchased, I am going to start building my new files around that code, in addition to other source code that I found for obtaining thermistor temperatures. The source code for obtaining thermistor temperatures can be found here: https://learn.adafruit.com/thermistor/using-a-thermistor.
If you visited the link provided, you will see that I am no longer going to be relying on thermistor tables, instead I will be generating an unknown amount of samples for both the heated bed and the extruder, and averaging the temperatures on the fly. Most likely I will have external current temperature variables for both the heated bed and the extruder, and gcode_process.c + h, will most likely have external target temperature variables for both the heated bed and the extruder. These four variables (bed_target_temp, extruder_target_temp, bed_current_temp, extruder_current_temp) will be monitored by the ADS1015 library, and this library will further control the pwm going to both heaters and additionally the extruder fan. By having the ADC source code control all of the heating and temperature related issues, I believe that will greatly simplify this port.
Many changes have occured since I last updated the archive in this thread. Instead of me describing all of the changes, I invite you to take a good look at the source. Sorry, but I am all typed out for now.
However I will say that it still needs some polishing to get it to where I want it for the next step.
EDIT: I suppose I should tell you that at this point, there are two serial objects. One is provided by SimpleTools and there is another initiated sergcode.c.
Two reasons really...
1) I can't access github with IE and I particularly don't like using Chrome, unless absolutely necessary.
2) And I never tried it before. Not really interested in learning all the ins and outs of github
And another reason.... For the changes I just made, I would spend hours and hours documenting it. Not worth my time.
You don't actually need to use a browser to clone fork and other wise work with repositories on github.
If you are working on any kind of code base bigger than 1 line that will evolve over time, especially if it is derived from another project in git, the git saves you so much headache.
Really, it's worth becoming a bit familiar with. Even if it's a project that will never leave your own computer.
Hmmmm......
Why would I lie.
OS = WindowsXP SP3 Highest browser avaiable for that OS = IE8 Security certificate expired
I familiar with using the github website, just not forking.
Even when I get this firmware going, I will have to go to github and review the original firmware changes, so I better be familiar with it
I avoided source code management systems for years. They were all universally horrible. Like MS Visual Source Safe, Rational/IBM Clear Case, and a bunch of others I forget. Basically more work than they are worth. Only used them when people paid me to:)
git changes all of that. There are only a few git commands you need to know to have it be a God send even on a very small project even if you never share or collaborate with anyone else: clone, add, commit, rm, mv, log, status, diff, branch, merge, checkout, pull, push.
Than again their are GUI interfaces if you are shy of the command line. I have never looked into those much. Apart from github that is.
It can be a lot to learn, but will surely pay off. Commit history is wonderful and, as heater said, can work as basic documentation of all your changes.
Here's some helpful links to get you going:
The very basics at the command line
Forking & Pull requests
Standalone Windows GUI
JetBrains (CLion) Integration - my preferred method
As mentioned earlier within this thread, I believe that timer.c is the main speed limitation for Teacup, which if I am not mistaken, maxes out at 20MHz. There are actually two timers that run, one for maintenance and another for stepping through the moves.
I was thinking about cog usage last night and that cogs might be running slim, however I believe, in order to make a much smoother and easier port of this firmware, each of these timers should probably have there own cog. By running each of these timers in their own cog, I believe it will pretty much eliminate the use of interrupts throughout the remaining source.
If anyone has input that they would like to share pertaining to the setup of these timers, then I am all ears.
EDIT: Additionally, it appears that all references to a memory barrier seem to pertain to a bug in one of the AVR libraries, so I suppose I could pretty much chuck memory_barrier.h out the window and all references to the file and it's functions. However, I believe I may be treading on thin ice and it may make it more difficult later to track down my own set of bugs
memory_barrier.h is all about disabling and enabling interrupts. Let's ignore that problem with the bug they mention.
If you consider that code running in the "background" and code running within an interrupt handler is actually two threads, which might well be running on two cogs on the Prop, then you see that disabling/enabling interrupts is actually a lock mechanism. Rather like claiming and releasing a lock on the Prop.
That of course is all about sharing data between threads without having them both access the same data at the same time and getting corrupt results. It removes data races.
Take that out and you will have a randomly failing program!
As the Prop has no interrupts we don't need to enable/disable them but I believe you will be running background and interrupt handler code on different COGs so you will no doubt need to acquire/release a lock instead to avoid data races.
Yea, that is also my belief, but their code is not going to perform the necessary task on the Propeller, so it is gone. There is no doubt that I will have to reestablish locking mechanisms, but I will cross that bridge, just as soon as I get the timer all straightened out. I will have to do some studying on the use of Propeller locks.
Presumably the code is sprinkled here and there with ATOMIC_START and ATOMIC_END wrapped around critical regions, where shared data is accessed.
Those are exactly the places you will probably want to acquire and release locks if those threads are run from separate COGs.
The correct solution is to keep that file and add #ifdef PROPELLER / #else around what will be used on the Propeller when you get to it. I.e the lock handling code.
As is often stated the aim should be to make as few changes to higher level code as possible. And this is a fine example.
However, I believe there will also be several instances where locks will not be needed. For instance, consider the following code, I believe this can be accomplished without a lock.
void setTimer(uint32_t delay) { uint16_t step_start = 0; // An interrupt would make all our timing calculations invalid, // so stop that here. cli(); CLI_SEI_BUG_MEMORY_BARRIER(); // Assume all steps belong to one move. Within one move the delay is // from one step to the next one, which should be more or less the same // as from one step interrupt to the next one. The last step interrupt happend // at OCR1A, so start delay from there. step_start = OCR1A; next_step_time = delay; // Now we know how long we actually want to delay, so set the timer. if(next_step_time < 65536) { // set the comparator directly to the next real step OCR1A = (next_step_time + step_start) & 0xFFFF; } else if(next_step_time < 75536) { // Next comparator interrupt would have to trigger another // interrupt within a short time (possibly within 1 cycle). // Avoid the impossible by firing the interrupt earlier. OCR1A = (step_start - 10000) & 0xFFFF; next_step_time += 10000; } else { OCR1A = step_start; } // Enable this interrupt, but only do it after disabling // global interrupts (see above). This will cause push any possible // timer1a interrupt to the far side of the return, protecting the // stack from recursively clobbering memory. TIMSK1 |= MASK(OCIE1A); }Source: Teacup Firmware
I would have to respectfully disagree. I believe that the file should be eliminated and new locks should be etablished where they are required, with Propeller specific functions for such matters.
Well OK, that can be true. If they are disabling interrupts so as not to disturb timing in some code sequence rather than protect shared data/resource access then you will not need that when running on separate COGs.
I have to respectfully disagree with your respectful disagreement:)
If there are cases of shared resource protection, I have not trawled through the code to check, then you should use the same file and #ifdef some Propeller equivalent code in there. That is the normal and expected practice.
At the end of the day, when you have everything tested and working you should be able to offer your changes back to the original project maintainer and he could merge then into his code base as a new supported platform. If you hack to much unnecessarily he is unlikely to want to do that.
Edit: Yes, and that lockxxx code should go into the same file as the the current ATOMIC operations.
Yea, that is what I am trying to say, and just eliminate memory_barrier.h completely, as well as ATOMIC_START and ATOMIC_END references, and hopefully it will work instead of causing me a lot of grief.