The major drawback to "_cogstart_PASM" and "_cogstart_C" is that they don't follow the convention of using the Spin name. For almost everything else if there's a Spin function "foo" then there's a C function "_foo" that does the same thing. You're proposing that instead of the Spin function "coginit" we use a C function named "_cogstart_something".
I think we should at least keep the "_coginit" prefix. There is some merit in having separate functions for starting PASM and C code (the Spin coginit function does double duty). How about "_coginit_c" and "_coginit_pasm" (all lower case) for the two functions? I'd also keep the same general prototype as Spin, with the minimal change to handle the different C stack layout. So I'd propose:
int _coginit_pasm(int cog, void *pasm_code, void *parameter);
int _coginit_c(int cog, void (*cfunc)(void *arg), void *parameter, void *stack_base, size_t stack_size);
(We did have "_cogstart" before, which was taken from PropGCC and had different parameters, but I think it'd be nice to aim for consistency in the naming).
The major drawback to "_cogstart_PASM" and "_cogstart_C" is that they don't follow the convention of using the Spin name. For almost everything else if there's a Spin function "foo" then there's a C function "_foo" that does the same thing. You're proposing that instead of the Spin function "coginit" we use a C function named "_cogstart_something".
I think we should at least keep the "_coginit" prefix. There is some merit in having separate functions for starting PASM and C code (the Spin coginit function does double duty). How about "_coginit_c" and "_coginit_pasm" (all lower case) for the two functions? I'd also keep the same general prototype as Spin, with the minimal change to handle the different C stack layout. So I'd propose:
int _coginit_pasm(int cog, void *pasm_code, void *parameter);
int _coginit_c(int cog, void (*cfunc)(void *arg), void *parameter, void *stack_base, size_t stack_size);
(We did have "_cogstart" before, which was taken from PropGCC and had different parameters, but I think it'd be nice to aim for consistency in the naming).
I already have a "_coginit_C" as well. I think using "_coginit_c" would be just too confusing
I'm okay to use any other name you like - just as long as it doesn't conflict with names that are already in use - i.e.:
_coginit()
_coginit_C()
_coginit_Spin()
I will use "_cogstart_Language()" for now. If you can come up with a better name, I am happy to switch to that.
Remember too that <propeller2.h> is a new file, so from a strict compatibility point of view it doesn't have to follow existing conventions of the compiler. Files that include <propeller2.h> could expect _coginit to be treated differently than files that don't include it. So for example we could certainly have:
// actual external function name
int _p2_coginit(int cog, void *pasm_code, void *param);
// standard define for propeller2.h users
#define _coginit(a, b, c) _p2_coginit((a), (b), (c))
I still think there's a real benefit to following the Spin example for the names of the functions.
Remember too that <propeller2.h> is a new file, so from a strict compatibility point of view it doesn't have to follow existing conventions of the compiler. Files that include <propeller2.h> could expect _coginit to be treated differently than files that don't include it. So for example we could certainly have:
// actual external function name
int _p2_coginit(int cog, void *pasm_code, void *param);
// standard define for propeller2.h users
#define _coginit(a, b, c) _p2_coginit((a), (b), (c))
I still think there's a real benefit to following the Spin example for the names of the functions.
The problem is that you are changing the parameter profile of the P1 functions with the same names for the P2, but are not changing the names.
But in C, it is not really feasible to have two functions with the same name but which accept different parameter profiles. My _coginit() function on the P2 does exactly what my _coginit() function did on the P1. But I cannot also have a different _coginit() function.
Remember too that <propeller2.h> is a new file, so from a strict compatibility point of view it doesn't have to follow existing conventions of the compiler. Files that include <propeller2.h> could expect _coginit to be treated differently than files that don't include it. So for example we could certainly have:
// actual external function name
int _p2_coginit(int cog, void *pasm_code, void *param);
// standard define for propeller2.h users
#define _coginit(a, b, c) _p2_coginit((a), (b), (c))
I still think there's a real benefit to following the Spin example for the names of the functions.
The problem is that you are changing the parameter profile of the P1 functions with the same names for the P2, but are not changing the names.
But in C, it is not really feasible to have two functions with the same name but which accept different parameter profiles. My _coginit() function on the P2 does exactly what my _coginit() function did on the P1. But I cannot also have a different _coginit() function.
You certainly can use macros though to redefine functions (see my example above). That is, it would be perfectly feasible to make propeller2.h have a _coginit macro so that programs that #include <propeller2.h> see the "_coginit" API as behaving differently from the _coginit in <catalina_cog.h>. I realize that this does create difficulties from a documentation perspective.
It looks like Catalina does not currently have any _cognew() function, so I think we could at least create _cognew() and _cognew_c() functions that do something similar to the Spin COGNEW (allocate a new COG and start it with PASM or C code). Let's start there...
I just noticed there is a name collision with my existing library code for _coginit().
I would therefore like to propose the following names, instead of _coginit() and _cogstart():
/* start PASM code in another COG */
int _cogstart_PASM(int cog, void *pgm, void *ptr);
/* start C code in another COG */
int _cogstart_C(void (*func)(void *), void *arg, void *stack_base, uint32_t stack_size);
This actually makes more sense, because some compilers may support starting code written in language "X" as well as C code, and in that case they might need both _cogstart_C() and _cogstart_X(). For instance, Catalina will also have _cogstart_Spin().
Apologies for not noticing this before
I like it.
One though: in the P1 world, PASM referred to a specific dialect of assembly. In the early PropGCC days, PASM wasn't available, only GAS. However, both of those dialects resulted in the same binary blob on the Propeller and could be loaded into a new cog in the same fashion. I would hate for someone to see "_cogstart_PASM", "_cogstart_C", "_cogstart_SPIN" and think that they're unable to load a GAS file. Or what about a C file compiled as cogc/ecogc?
I don't have any brilliant ideas... here's some I've thought of but don't like very much:
* _cogstart_Native - This doesn't distinguish itself very well from C/C++ or any other compiled-to-native compiler
* _cogstart_Bare (or cogstart_Baremetal) - Same problem as above
* _cogstart - Without any suffix... it's your "default" cogstart function. I have the fewest problems with this one.
Also, I just noticed all the replies after Ross's initial post about cogstart_*. I have no strong preference on the naming convention other than the PASM suffix. I'll let you two duke it out
Can anyone clarify what _lockchk() and _cogchk() are expected to return?
Well, since it seems to be up to me to decide, here is what I propose:
_cogchk() will return 1 if the specified cog is busy (i.e. executing), 0 if not. _lockchk() will return the cog number if the lock has been allocated to a cog, -1 if not.
Also, for completeness, we should define what the other lock functions will return:
_locknew() - allocates a lock and returns the lock number if one is available, otherwise -1. _lockret() - returns a lock, and returns nothing. _locktry() - implements P2 lock semantics, and returns 1 if the lock was successfully captured, 0 if not. _lockrel() - implements P2 lock semantics, and returns nothing.
I also intend to add functions that reproduce the P1 style lock semantics, which are much more useful than the P2 ones:
_lockset() - implements P1 lock semantics to set the lock, and returns 1 if the lock was successfully set, 0 if not _lockclr() - implements P1 lock semantics to clear the lock, and returns nothing.
_acquire_lock() - implements P1 lock semantics, but does not return until the lock is successfully captured. Returns nothing. _release_lock() - implements P1 lock semantics, and returns nothing (i.e. same as lockclr()).
I agree with you that the new lock semantics are not really nice. I do not know why this had to be changed. My guess is that I could not follow chips thought there, it might has a reason, I just don't see it.
I agree with you that the new lock semantics are not really nice. I do not know why this had to be changed. My guess is that I could not follow chips thought there, it might has a reason, I just don't see it.
Mike
The LOCKs were modified to be suitable for use in the debugging scheme, so that if a cog is stopped or restarted, it loses ownership of any LOCK that it held. This keeps debugging from getting hung up.
That's a nice feature. I had to take care to clear locks to prevent lockup when I stopped a cog in Spinix on the P1. Of course, that shouldn't have required changing the names and functions of the lock instructions. The P1 macros in propeller2.h should help make code portable between the P1 and the P2.
Not a big fan of that last one. Would __test_pin be alright? I know it breaks the pattern... So maybe __pintest? It's just so ambiguous.
__pinf isn't great... I always prefer longer names (__pin_float) but I don't consider this half as bad as __pin.
All the others seem great.
_pinf might be pin false ? - what about _pints to tristate the pin ?
_pin(int pin) is ok, with inferred read by the call usage, but it could also be _pinrd(int pin) if you wanted to make it more clearly a read operation.
_pinnot() may be clearer as _pintog() as toggle is more general use, and self explanatory ?
__pinf isn't great... I always prefer longer names (__pin_float) but I don't consider this half as bad as __pin.
I'm ok to revise the names. I agree they don't convey their meaning very well. Assuming I am right about the "f" in _pinf() meaning "float", I think it would be better to have:
Aha! I just found (in a previous page of this thread) that "pinf()" does indeed mean "fltl".
But why dow we have that one but not also "flth"?
I think the idea of those niche opcodes, was to allow glitch-free exit, with a DIRxx opcode, but they are unusual.
FLTxx opcodes all clear the DIRN bit, and also change the OUT bit (I presume after the pin tristates)
Subsequent to that, you can use a DIRxx opcode to change only the DIR bit, and the pin drive is now from whatever OUT was allocated before.
If you have fltl, flth, you probably also need a dirh to be used later, to apply that previous OUT level.
To confuse things for new users even more, there are light pullups and pulldowns, so someone could use a flth command to float a pin with a light pulldown, aka `pin floats low`...
or a fltl with a pin with a light pullup, to `float the pin hi`...
To confuse things for new users even more, there are light pullups and pulldowns, so someone could use a flth command to float a pin with a light pulldown, aka `pin floats low`...
or a fltl with a pin with a light pullup, to `float the pin hi`...
I don't think the pullups are active when floating, but I could be wrong. My understanding of the resistors is that they are only applied when dong a drvh or drvl (ie when the DIR signal is high)
So as fltl and flth are setting the output state (OUT bit) ready for if you toggled the DIR bit, but I don't think any current or resistors are in play when floating.
To confuse things for new users even more, there are light pullups and pulldowns, so someone could use a flth command to float a pin with a light pulldown, aka `pin floats low`...
or a fltl with a pin with a light pullup, to `float the pin hi`...
I don't think the pullups are active when floating, but I could be wrong. My understanding of the resistors is that they are only applied when dong a drvh or drvl (ie when the DIR signal is high)
So as fltl and flth are setting the output state (OUT bit) ready for if you toggled the DIR bit, but I don't think any current or resistors are in play when floating.
Happy to be corrected
To make a high pull-up, you need to set the %HHH bits via WRPIN to %001..%110 for 1.5k/15k/150k/1mA/100uA/10uA and do a DRVH (or set that pin's DIR and OUT bits high). It will be outputting a high, but a weak one. Reading the pin's IN bit will return its input state.
Eric and RossH, for what it's worth, I had changed PIN(pin) to PINR(pin) in Spin2 for reading the pin, in order to avoid hogging the name "PIN".
That's not such a problem in C, because the convention is to use a leading underscore to indicate system reserved names, so "pin" is still available for applications to use. But if Eric agrees to change to _pinr() for consistency with Spin, I am happy to do so.
Eric and RossH, for what it's worth, I had changed PIN(pin) to PINR(pin) in Spin2 for reading the pin, in order to avoid hogging the name "PIN".
That's not such a problem in C, because the convention is to use a leading underscore to indicate system reserved names, so "pin" is still available for applications to use. But if Eric agrees to change to _pinr() for consistency with Spin, I am happy to do so.
Sure, I think keeping consistency with Spin is worthwhile.
Of all of them I think the "_pin(int pin)" API to read a pin is probably the least confusing, unless you incorrectly ever tried to assign it to a value like _pin(3)=1 or something. But in C you don't really use that type of syntax, only C++. So you can pretty much figure out in the code what it means given there are no other arguments passed in and there is no return value. However the other ones like _pinh and _pinl are harder to figure out right away. Do they return true if the pins are high or low, or do they actually set them high or low?
IMO this confusion has purely come about due to the truncation of the API name by too much. _pinsetl _pinseth or pindrvl pindrvh would have been easier to understand right away but they are 3 char's longer than _pinh, _pinl. Big deal, still way less than the Arduino guys with their API:
used to COBOL I do prefer long speaking names compared to short ones. Still do this in most languages.
I even argued with chip about his obsession of keeping PASM key words under 8 char. Screens are quite wider now and, hmm, what difference makes another tab?
The whole shortness thing came up with them dreaded C programmers, heck they even could not come up with a name for the language, just a letter.
Actually a digitalWrite(pin, value) looks way better to me as _pinw(int pin, int val) and why val and not value. It is just WRONG.
At least _pinRead(pin), _pinFloat(pin) etc and just name them after what they are doing, human readable. Same goes for Spin, @cgracey.
We do not have 40 character displays anymore, even COBOL has free format since 30+ years after they dropped the punch cards and the 72 column rules.
The WORST thing are variables named a,b,c,d,e or other single and double letter stuff like A1, A2. Reading this chip?, yes you are doing this a lot.
One can not even replace them with search and replace. Except maybe in Visual Studio with support for the language where you can rename variables inside their scope.
Almost all Editors I use offer code completion and programming is NOT typing as fast as you can and produce the shortest source file.
a "MULTIPLY unit-price BY unit-count GIVING unit-total" needs not really a comment, a "t = u * p" does need one
But who listens to people fighting with code other people wrote 20+ years ago and died away.
@JonnyMac knows that. And that is one reason everybody loves his code. The second one is that it is usual bug free and easy to read/understand.
Because he has a admirable coding style most coders I know could learn a lot from. Readable code.
You write it once but it will get read often, even by yourself. Have you looked lately at some source you wrote 5-10 years ago? Do you understand what you where thinking when you wrote it?
Or chips Video driver for the P1. Constants to configure the driver. Readability of the constant name would make things easier to understand. IT IS A CONSTANT in the SOURCE god help.
Comments
I think we should at least keep the "_coginit" prefix. There is some merit in having separate functions for starting PASM and C code (the Spin coginit function does double duty). How about "_coginit_c" and "_coginit_pasm" (all lower case) for the two functions? I'd also keep the same general prototype as Spin, with the minimal change to handle the different C stack layout. So I'd propose:
(We did have "_cogstart" before, which was taken from PropGCC and had different parameters, but I think it'd be nice to aim for consistency in the naming).
I already have a "_coginit_C" as well. I think using "_coginit_c" would be just too confusing
I'm okay to use any other name you like - just as long as it doesn't conflict with names that are already in use - i.e.:
I will use "_cogstart_Language()" for now. If you can come up with a better name, I am happy to switch to that.
I still think there's a real benefit to following the Spin example for the names of the functions.
The problem is that you are changing the parameter profile of the P1 functions with the same names for the P2, but are not changing the names.
But in C, it is not really feasible to have two functions with the same name but which accept different parameter profiles. My _coginit() function on the P2 does exactly what my _coginit() function did on the P1. But I cannot also have a different _coginit() function.
You certainly can use macros though to redefine functions (see my example above). That is, it would be perfectly feasible to make propeller2.h have a _coginit macro so that programs that #include <propeller2.h> see the "_coginit" API as behaving differently from the _coginit in <catalina_cog.h>. I realize that this does create difficulties from a documentation perspective.
It looks like Catalina does not currently have any _cognew() function, so I think we could at least create _cognew() and _cognew_c() functions that do something similar to the Spin COGNEW (allocate a new COG and start it with PASM or C code). Let's start there...
I like it.
One though: in the P1 world, PASM referred to a specific dialect of assembly. In the early PropGCC days, PASM wasn't available, only GAS. However, both of those dialects resulted in the same binary blob on the Propeller and could be loaded into a new cog in the same fashion. I would hate for someone to see "_cogstart_PASM", "_cogstart_C", "_cogstart_SPIN" and think that they're unable to load a GAS file. Or what about a C file compiled as cogc/ecogc?
I don't have any brilliant ideas... here's some I've thought of but don't like very much:
* _cogstart_Native - This doesn't distinguish itself very well from C/C++ or any other compiled-to-native compiler
* _cogstart_Bare (or cogstart_Baremetal) - Same problem as above
* _cogstart - Without any suffix... it's your "default" cogstart function. I have the fewest problems with this one.
Also, I just noticed all the replies after Ross's initial post about cogstart_*. I have no strong preference on the naming convention other than the PASM suffix. I'll let you two duke it out
Well, since it seems to be up to me to decide, here is what I propose:
_cogchk() will return 1 if the specified cog is busy (i.e. executing), 0 if not.
_lockchk() will return the cog number if the lock has been allocated to a cog, -1 if not.
Also, for completeness, we should define what the other lock functions will return:
_locknew() - allocates a lock and returns the lock number if one is available, otherwise -1.
_lockret() - returns a lock, and returns nothing.
_locktry() - implements P2 lock semantics, and returns 1 if the lock was successfully captured, 0 if not.
_lockrel() - implements P2 lock semantics, and returns nothing.
I also intend to add functions that reproduce the P1 style lock semantics, which are much more useful than the P2 ones:
_lockset() - implements P1 lock semantics to set the lock, and returns 1 if the lock was successfully set, 0 if not
_lockclr() - implements P1 lock semantics to clear the lock, and returns nothing.
_acquire_lock() - implements P1 lock semantics, but does not return until the lock is successfully captured. Returns nothing.
_release_lock() - implements P1 lock semantics, and returns nothing (i.e. same as lockclr()).
Sorry, RossH. I started to answer this yesterday and got sidetracked.
Your proposals in the post above all seem correct to me.
Mike
The LOCKs were modified to be suitable for use in the debugging scheme, so that if a cog is stopped or restarted, it loses ownership of any LOCK that it held. This keeps debugging from getting hung up.
This held me up two days and now I just require the lock number to use as parameter.
usable but not nice.
Mike
Here is what I am currently assuming:
*** this is the one that is really not obvious, but I may have interpreted the others wrongly as well
__pinf isn't great... I always prefer longer names (__pin_float) but I don't consider this half as bad as __pin.
All the others seem great.
_pinf might be pin false ? - what about _pints to tristate the pin ?
_pin(int pin) is ok, with inferred read by the call usage, but it could also be _pinrd(int pin) if you wanted to make it more clearly a read operation.
_pinnot() may be clearer as _pintog() as toggle is more general use, and self explanatory ?
I'm ok to revise the names. I agree they don't convey their meaning very well. Assuming I am right about the "f" in _pinf() meaning "float", I think it would be better to have:
However, I'm actually ok with just having _pin(), because I think code like:
reads better than:
However, that's just personal taste. If the consensus is to change it, I'll change it.
But why dow we have that one but not also "flth"?
I think the idea of those niche opcodes, was to allow glitch-free exit, with a DIRxx opcode, but they are unusual.
FLTxx opcodes all clear the DIRN bit, and also change the OUT bit (I presume after the pin tristates)
Subsequent to that, you can use a DIRxx opcode to change only the DIR bit, and the pin drive is now from whatever OUT was allocated before.
If you have fltl, flth, you probably also need a dirh to be used later, to apply that previous OUT level.
To confuse things for new users even more, there are light pullups and pulldowns, so someone could use a flth command to float a pin with a light pulldown, aka `pin floats low`...
or a fltl with a pin with a light pullup, to `float the pin hi`...
I don't think the pullups are active when floating, but I could be wrong. My understanding of the resistors is that they are only applied when dong a drvh or drvl (ie when the DIR signal is high)
So as fltl and flth are setting the output state (OUT bit) ready for if you toggled the DIR bit, but I don't think any current or resistors are in play when floating.
Happy to be corrected
To make a high pull-up, you need to set the %HHH bits via WRPIN to %001..%110 for 1.5k/15k/150k/1mA/100uA/10uA and do a DRVH (or set that pin's DIR and OUT bits high). It will be outputting a high, but a weak one. Reading the pin's IN bit will return its input state.
That's the way I implemented them as well. I too am not a huge fan of the names, but I suppose whatever we pick will be confusing to some.
That's not such a problem in C, because the convention is to use a leading underscore to indicate system reserved names, so "pin" is still available for applications to use. But if Eric agrees to change to _pinr() for consistency with Spin, I am happy to do so.
Sure, I think keeping consistency with Spin is worthwhile.
IMO this confusion has purely come about due to the truncation of the API name by too much. _pinsetl _pinseth or pindrvl pindrvh would have been easier to understand right away but they are 3 char's longer than _pinh, _pinl. Big deal, still way less than the Arduino guys with their API:
digitalWrite(pin, value)
I even argued with chip about his obsession of keeping PASM key words under 8 char. Screens are quite wider now and, hmm, what difference makes another tab?
The whole shortness thing came up with them dreaded C programmers, heck they even could not come up with a name for the language, just a letter.
Actually a digitalWrite(pin, value) looks way better to me as _pinw(int pin, int val) and why val and not value. It is just WRONG.
At least _pinRead(pin), _pinFloat(pin) etc and just name them after what they are doing, human readable. Same goes for Spin, @cgracey.
We do not have 40 character displays anymore, even COBOL has free format since 30+ years after they dropped the punch cards and the 72 column rules.
The WORST thing are variables named a,b,c,d,e or other single and double letter stuff like A1, A2. Reading this chip?, yes you are doing this a lot.
One can not even replace them with search and replace. Except maybe in Visual Studio with support for the language where you can rename variables inside their scope.
Almost all Editors I use offer code completion and programming is NOT typing as fast as you can and produce the shortest source file.
a "MULTIPLY unit-price BY unit-count GIVING unit-total" needs not really a comment, a "t = u * p" does need one
But who listens to people fighting with code other people wrote 20+ years ago and died away.
@JonnyMac knows that. And that is one reason everybody loves his code. The second one is that it is usual bug free and easy to read/understand.
Because he has a admirable coding style most coders I know could learn a lot from. Readable code.
You write it once but it will get read often, even by yourself. Have you looked lately at some source you wrote 5-10 years ago? Do you understand what you where thinking when you wrote it?
Or chips Video driver for the P1. Constants to configure the driver. Readability of the constant name would make things easier to understand. IT IS A CONSTANT in the SOURCE god help.
No two letters
fp long whatever
go figure out what fp means
can you see my problem?
am I over reacting?
Mike