How best to "detect a min pulse width" using C18 + 18F1330 pic

In article <i7q936$au0$1@speranza.aioe.org>, Mel  <mwilson@the-wire.com> wrote:
>Michael Kellett wrote:
>
>> Are you really sure that it is always best to put every single number into
>> a header file ?
>> (I often do but) I'm becoming more and more convinced that a magic number
>> used only once is (often) better used directly and commented.
>
>It's very rare for a magic number to be used only once.  Even if 0x34 is
>used once as the length of the FOO packet, there's every chance that it gets
>used again as a bit mask initializing the UART, or some such thing.
>Defining a name like FOO_PKT_LEN gives your maintenance programmer a search
>target that's just as plain as searching for a comment, and assures you that
>you and your program agree on the definition, which a comment does not.

Not "very rare".
E.g. all the magic numbers associated with e.g. Unix commands
are essentially used only in one place in a definition like:

"
/* Buried under three levels of including h-files: */
#define __NR_EXIT_ 1
#define __NR_EXIT  __NR_EXIT_

sysexit( p1 )
{
    __sys__( p1, _NR_EXIT );
}

"

There are reasons to doubt the sanity of this.

>
>> And also that magic numbers which are not global should be defined in the
>> source file in which they are used.
>> If you don't do this then should you supoport each source file with (at
>> least) TWO header files, one for global stuff and one for local stuff ?
>
>This is true.  I have no problem with definitions migrating out of the
>program preamble into the header file as the system grows.
>
>I got this in spades in firmware for a device I had to interface with.  The
>code was full of
>
>    set_bit (flags, 3);
>or
>    clr_bit (flags2, 5);
>
>to set or clear individual bits in a couple of bytes of generic boolean
>flags.  And, for added spice,
>
>    if (flags & 0x04)
>
>to make decisions in the code, so that the connections with the sets and
>clrs was as obscure as possible.  After several days figuring out how the
>program actually did what it did, we wrote a Python program to scan for
>these constructs and replace them with
>    set_bit (flags, BUFFER_FULL)
>or
>    if (tst_bit (flags2, INPUT_COMPLETE))
>
>The third thing we did was to change the Python program to work from files
>of definitions, so we could run it over that programmer's other source
>programs.
>
>Not to demean the engineer who wrote those programs.  His designs and the
>programs he wrote for them were accurate, and good or even brilliant in most
>ways.  Just that by the time he understood bit-for-bit what to do, he wrote
>that down and moved on to make the next working product.  It was after, when
>he'd moved on and the product requirements had changed, that we found we had
>a problem.

In the end the magic bits have to be documented.

I dislike

#define 0x0020    MASK_BAUDRATE_MAX_LEFT_TIMING_OPERAND_BITS_REAMING
#define 0x0040    MASK_BAUDRATE_MAX_RIGHT_TIMING_OPERAND_BITS_REAMING

and then I have to code like
"
x= *(io_port_for_reaming)
if ( x &  ( MASK_BAUDRATE_MAX_LEFT_TIMING_OPERAND_BITS_REAMING |
    MASK_BAUDRATE_MAX_RIGHT_TIMING_OPERAND_BITS_REAMING ) )
{
"
(Unless coding standards force me to
        if ( 0 != ( x & ...
don't get me started on that )

The author apparently has traded the explanation (that needed 3 lines)
by one incomprehensible and unusable name.

I reserve the right to use 0x0060 here and of course document properly
what is going on here and what the bits mean.

I do demean the engineer who wrote your programs though. I've met
the likes of him and they wasted more and more precious time, than
they saved for themselves. You've added more value to the programs
than they had originally. Unless of course they came with a
comprehensive set of tests, then it may be about a wash.
(I mean the original value and the value added.)
But undoubtedly the author has skimped on tests too.

>
>       Mel.
>>
>> Michael Kellett
>

Groetjes Albert

--
-- 
Albert van der Horst, UTRECHT,THE NETHERLANDS
Economic growth -- being exponential -- ultimately falters.
albert@spe&ar&c.xs4all.nl &=n http://home.hccnet.nl/a.w.m.van.der.horst

On 27/09/2010 14:35, Rich Webb wrote:
> On Mon, 27 Sep 2010 08:45:52 +0100, "Michael Kellett"
> <nospam@nospam.com>  wrote:
>
>>
>> "Rich Webb"<bbew.ar@mapson.nozirev.ten>  wrote in message
>> news:cr0s96pb275hapgd3ci2q2sf4lluhpe9sl@4ax.com...
>>> On Sat, 25 Sep 2010 20:44:57 +0800, "Dennis"<invalid@nowhere.com>
>>> wrote:
>>>
>>> Also, strongly recommend that you get in the habit of *never* using
>>> literal "magic numbers" (e.g., to test against gBreakLength or to set
>>> gWatchDog). Abstract those out to a header file and do something like
>>> #define MIN_BREAK_LENGTH 10
>>> #define WDOG_EVENT       125
>>>
>>> --
>>> Rich Webb     Norfolk, VA
>>
>> Rich,
>>
>> Are you really sure that it is always best to put every single number into a
>> header file ?
>> (I often do but) I'm becoming more and more convinced that a magic number
>> used only once is (often) better used directly and commented.
>> And also that magic numbers which are not global should be defined in the
>> source file in which they are used.
>> If you don't do this then should you supoport each source file with (at
>> least) TWO header files, one for global stuff and one for local stuff ?
>
> Some things, like register-dependent values, will typically be defined
> in the device header file or are fixed properties from the datasheet.
> Others, that are not likely to change, could be tagged either in a
> header file or in the source itself. E.g.,
>
>    UART0_LCR = 0x00000083;		// divisor unlock
>    UART0_DLM = 3;			// 4800 w/pclk 14.7, prsc 1
>    UART0_DLL = 0;
>
> is clear enough that there's not a huge benefit to #define'ing it.
>
> But where the numbers may change during development or where they can't
> be derived from the datasheet, like the break-length test value, then
> I'd strongly recommend sticking them in a header file.
>
> WRT using two headers: yes, where it makes sense: one header that other
> parts of the program need to know about and one that contains only local
> values. If there are only a few of the latter, collecting them together
> at the top of the source file achieves the same results.
>

There is generally no point in having a header file that is only used by 
a single module.  It is needless complication and destroys the 
interface/implementation arrangement you get by only having global 
interface data in header files.  The exception is if the module-private 
header can be re-used between projects but you can't re-use the actual 
implementation source file.

I fully agree with you about when to put magic numbers directly in the 
code, and when to use #define's (or static consts or enums if possible). 
  But if a #define'd symbol is only useful within a single module, then 
it should be defined in that module and nowhere else.

On 9/27/2010 12:56 PM, David Brown wrote:

> [snip]
> There is generally no point in having a header file that is only used by
> a single module. It is needless complication and destroys the
> interface/implementation arrangement you get by only having global
> interface data in header files. The exception is if the module-private
> header can be re-used between projects but you can't re-use the actual
> implementation source file.
>
> I fully agree with you about when to put magic numbers directly in the
> code, and when to use #define's (or static consts or enums if possible).
> But if a #define'd symbol is only useful within a single module, then it
> should be defined in that module and nowhere else.
>

I'd even go farther and say that magic numbers that are only being used 
in one part of the code that they should be defined right there at the 
point of use.  If I've got a bit-banged EEPROM then there ought to be 
one spot in my code that contains a few #define constants followed 
immediately by the write_eeprom and read_eeprom functions.

-- 
Rob Gaddi, Highland Technology
Email address is currently out of order

On 27.09.2010 22:07, Albert van der Horst wrote:
> In article<i7q936$au0$1@speranza.aioe.org>, Mel<mwilson@the-wire.com>  wrote:

> E.g. all the magic numbers associated with e.g. Unix commands
> are essentially used only in one place in a definition like:

And their definitions are all collected in _one_ central place.  Where 
you get the overview of used numbers, and can _easily_ pick a new one, 
if needed, without any danger that it has already been used in some 
other file.

Basically, the __NR_* macros are part of the interface specification of 
  the __sys() macro/function, and thus belong to the same header file 
that defines __sys().

> There are reasons to doubt the sanity of this.

So let's see if you're going to name a few of them here...

> In the end the magic bits have to be documented.

Indeed.  And assigning self-evident, meaningful names to them is how you 
do that.

> I dislike
>
> #define 0x0020    MASK_BAUDRATE_MAX_LEFT_TIMING_OPERAND_BITS_REAMING
> #define 0x0040    MASK_BAUDRATE_MAX_RIGHT_TIMING_OPERAND_BITS_REAMING

Apparently you dislike it so thoroughly you even completely forgot how 
to do it.  That's about the only explanation how you could have gotten

#define MASK_BAUDRATE_MAX_LEFT_TIMING_OPERAND_BITS_REAMING  0x0020
#define MASK_BAUDRATE_MAX_RIGHT_TIMING_OPERAND_BITS_REAMING 0x0040

so disastrously wrong...

On Sep 28, 2:59=A0pm, Hans-Bernhard Br=F6ker <HBBroe...@t-online.de>
wrote:
> On 27.09.2010 22:07, Albert van der Horst wrote:
>
> .....
>
> > I dislike
>
> > #define 0x0020 =A0 =A0MASK_BAUDRATE_MAX_LEFT_TIMING_OPERAND_BITS_REAMIN=
G
> > #define 0x0040 =A0 =A0MASK_BAUDRATE_MAX_RIGHT_TIMING_OPERAND_BITS_REAMI=
NG
>
> Apparently you dislike it so thoroughly you even completely forgot how
> to do it. =A0That's about the only explanation how you could have gotten
>
> #define MASK_BAUDRATE_MAX_LEFT_TIMING_OPERAND_BITS_REAMING =A00x0020
> #define MASK_BAUDRATE_MAX_RIGHT_TIMING_OPERAND_BITS_REAMING 0x0040
>
> so disastrously wrong...

He clearly meant the style, not the particular syntax - and I
agree 100% with his assessment.
 Only modern day programmers who cannot take real
advantage of lower level programming - and being true C writers
don't even know how and when to write comments - can have
something good to say about such a ridiculous naming style.

Dimiter

------------------------------------------------------
Dimiter Popoff               Transgalactic Instruments

http://www.tgi-sci.com
------------------------------------------------------
http://www.flickr.com/photos/didi_tgi/sets/72157600228621276/

Didi <dp@tgi-sci.com> wrote:
> He clearly meant the style, not the particular syntax - and I
> agree 100% with his assessment.
> Only modern day programmers who cannot take real
> advantage of lower level programming - and being true C writers
> don't even know how and when to write comments - can have
> something good to say about such a ridiculous naming style.

It's all fine and dandy until 0x0400 means different things for
different portions of an algorithm and then you see a nasty block of
code using the hard constant all over. Now you can't simply search and
replace for 0x0400 since you'll contaminate other sections of code
that happend to use the same constant, but do something completely
different with it.

Besides, you'd use enums instead of #defines since enums are usually
understood by the debugger. The debugger can present to you the
actual enum name instead of a random constant when viewing variables
containing them.

And for the same reason you don't see an assembly instruction mnenomic like:

add_a_left_register_to_right_register_and_store_into_left_register1 %eax, %ecx

you don't see atrocities with #define.

-pete