Searching \ for '[PIC]: Code Commenting, was: LCD problems revisite' in subject line. ()
Make payments with PayPal - it's fast, free and secure! Help us get a faster server
FAQ page: www.piclist.com/techref/io/lcd/pic.htm?key=lcd
Search entire site for: 'Code Commenting, was: LCD problems revisite'.

Exact match. Not showing close matches.
PICList Thread
'[PIC]: Code Commenting, was: LCD problems revisite'
2001\03\07@220506 by myke predko

flavicon
face
Hmmmm....

I guess I should have jumped in a bit earlier - I didn't think the emails
would get as angry as they did so quickly.

Thanx to James for taking the time to explain the code instruction by
instruction.


Looking over the code, I would probably format/document things differently -
but not that differently.  The (apparently) offending code (formatted for a
monospace font) is:

 clrf   FSR      ;  Output the Message
OutLoop
 movf   FSR, w   ;  Get the Offset to Output
 incf   FSR
 call   Message
 iorlw  0        ;  At the End of the Message?
 btfsc  STATUS, Z
  goto  Loop     ;  Yes - Equal to Zero
 call   SendCHAR ;  Output the ASCII Character
 goto   OutLoop

What is interesting is, that the following code, which is critical to the
entire operation of the function was not included in the quotes:

Loop              ;  Loop Forever when Done
 goto   Loop


;  Subroutines
Message           ;  Message to Output
 addwf  PCL      ;  Output the Characters
 dt     "Hello", 0



and I agree, that the first and last comments in the code above are
confusing and I probably should have improved them.  For the first comment,
I should have commented it as something like:

;  Output the "Message"
;  using the FSR as the
;  index into the table.

as using FSR is non-standard and probably not good practice if you are new
to working with the PICmicro.  I tend to use the FSR because of experiences
with early (GI) PICs where if you weren't using indexed addressing, FSR was
another file register for your use.

My only excuse to this is that the purpose of this code is to demonstrate
the two wire interface - not to demonstrate how to read from a table.

As for the last comment (";  Output the Characters"), I would probably
delete it as it really doesn't make any sense.

Other than that (other than adding ", f" to the end of the "incf FSR"
instruction) I would leave the code alone.


This brings up the question, how should code be "properly" documented?
Personally, I prefer minimal comments so that the operation of the code can
be seen easily without being distracted by comments.

My personal feeling is that if the code isn't written in such a way that if
you have to have it explained, then it can't be that
efficient/intuitive/helpful.  Going back to this case, I feel like I was
successful because a number of people were able to explain what the code was
doing.  On the other hand, I was unsuccessful as a "newbie" wasn't able to
understand what I was doing and got frustrated.


I was surprised at the comments regarding PCLATH with the table read - I
deliberately left the general case *out* to avoid making the code confusing.
Again, the primary purpose of this code was to demonstrate how to interface
with an LCD using two wires, NOT explain how to read from a table.


So, the overall question is, what is the *best* way of writing code to show
concepts to others?  I am very interested in this topic (and I have very
strong opinions on what is the best way to write/document/test code).

This was discussed four or so years ago on the list with a lot of anger and
finger pointing and really degenerated into a non-useful exercise.

To try and keep down the hard feelings and keep the conversation
constructive, I would ask that people present *good* examples of source
code/documentation only.  If you want to send me examples of what you think
is good code, I'll be more than happy to discuss it with you and I hope that
this could be something very positive for the PICList as well as everybody
on it.

myke

--
http://www.piclist.com hint: To leave the PICList
spam_OUTpiclist-unsubscribe-requestTakeThisOuTspammitvma.mit.edu


2001\03\08@090545 by Olin Lathrop

face picon face
{Quote hidden}

can
> be seen easily without being distracted by comments.

Comments are there to help, not distract.  If everything is properly and
consistantly formatted then they are not "in the way" if you prefer to
ignore them.  To facilitate this, I consistantly use column 10 for opcodes,
18 for parameters, and 30 for comments.  I admit I haven't found a good way
to deal with assembly directives.

> So, the overall question is, what is the *best* way of writing code to
show
> concepts to others?  I am very interested in this topic (and I have very
> strong opinions on what is the best way to write/document/test code).

Since you asked, here is how I would write and format this code:


;
;   Write the message to the display.
;
        clrf    fsr         ;init message index
outloop  unbank              ;back here each new message character
        movf    fsr, w      ;get the index for this character
        incf    fsr         ;increment the index for next time
        mcallr  message, reg0 ;get the indexed message char in REG0
        movf    reg0        ;set Z if this char is null terminator
        skip_nz             ;got valid message char, not terminator ?
        goto    loop        ;done writing the message
        gcall   sendchar    ;send char in REG0 to the display
        goto    outloop     ;back to do next message character
;
;********************
;
;   Local subroutine MESSAGE
;
;   Return the indexed message character in W.  The 0-N character
;   index is in FSR, which is preserved.
;
message  locsub, noregs
        movlw   low msg     ;get low byte of table start address
        addwf   fsr, w      ;make table entry address low byte
        skip_ncarr          ;propagate carry to high address byte
        incf    pclath
        movwf   pcl         ;jump the the selected table entry

msg      dt      "Hello"     ;the message string to send
        dt      0           ;string terminator


This isn't exactly the same code you wrote.  I modified it slightly to fit
the way I write PIC code, which includes my bank switching and subroutine
linkage conventions.  I made the assumption that this code was neither speed
critical nor extremely tight on space (as is the case with 95% or more of
all code).  It is therefore far more important to make it robust and
maintainable than super fast and small.  For example, 256 word program
memory boundaries can now be ignored, which was not the case for your code.
Someone can come by in two years and add code above this snippet without
having to worry.  The only restriction is that all this code be on the same
code page.  I use linker sections to guarantee that all code within a module
is on the same page.  If I was writing this from scratch, I would have used
general table fetching subroutines, but it would have been messy to include
the source for them here.  I haven't tried to assemble this snippet above,
so it may contain typos, etc.

All the macros, general registers, and subroutine linkage conventions I used
above are defined in file STD.INS.ASPIC which can be found at
http://www.embedinc.com/pic.

And yes, I really do write all my code with this same general style and
level of commenting.


*****************************************************************
Olin Lathrop, embedded systems consultant in Devens Massachusetts
(978) 772-3129, .....olinKILLspamspam@spam@embedinc.com, http://www.embedinc.com

--
http://www.piclist.com hint: PICList Posts must start with ONE topic:
[PIC]:,[SX]:,[AVR]: ->uP ONLY! [EE]:,[OT]: ->Other [BUY]:,[AD]: ->Ads


2001\03\08@093318 by Olin Lathrop

face picon face
{Quote hidden}

I just realized that I changed the subroutine to take FSR as the index (as
documented), but forgot to go back and update the calling code.  Oh well,
that's what I get for letting people see my untested code.  However, the
point of this code was the documentation style, which is still valid.


*****************************************************************
Olin Lathrop, embedded systems consultant in Devens Massachusetts
(978) 772-3129, olinspamKILLspamembedinc.com, http://www.embedinc.com

--
http://www.piclist.com hint: PICList Posts must start with ONE topic:
[PIC]:,[SX]:,[AVR]: ->uP ONLY! [EE]:,[OT]: ->Other [BUY]:,[AD]: ->Ads


2001\03\08@104439 by Bruce Smith

picon face
Olin,
Wow as a newbie, your code is very understandable. Do you have a site with
more of that style code.

{Original Message removed}

2001\03\08@105445 by David VanHorn

flavicon
face
>> message  locsub, noregs
>>          movlw   low msg     ;get low byte of table start address
>>          addwf   fsr, w      ;make table entry address low byte
>>          skip_ncarr          ;propagate carry to high address byte
>>          incf    pclath
>>          movwf   pcl         ;jump the the selected table entry
>>
>> msg      dt      "Hello"     ;the message string to send
>>          dt      0           ;string terminator


The key point that I see here, is that the comments should say WHY you're
doing what you're doing, as they do here.
Any idiot can read the ASM code, but IMHO, the comments should provide the
reasoning behind the code.


--
Dave's Engineering Page: http://www.dvanhorn.org
Where's dave? http://www.findu.com/cgi-bin/find.cgi?kc6ete-9

--
http://www.piclist.com hint: PICList Posts must start with ONE topic:
[PIC]:,[SX]:,[AVR]: ->uP ONLY! [EE]:,[OT]: ->Other [BUY]:,[AD]: ->Ads


2001\03\08@110855 by Sam Linder

flavicon
face
I agree with both Olin and Scott regarding the level of documentation that
should be present in both C and ASM programs. There should be an overall
description of what the chunk of code is doing and there should also be line
by line comments as necessary to help clarify what each part of the code is
doing (especially in ASM). For my money, I would always rather have too many
comments than not enough. Too many "guru's" out there think their code is so
"self-documenting" that they are blind to the fact that no one else can
initially understand it without a careful perusal of each line of code.

{Original Message removed}

2001\03\08@111659 by David VanHorn

flavicon
face
At 08:08 AM 3/8/01 -0800, Sam Linder wrote:
>I agree with both Olin and Scott regarding the level of documentation that
>should be present in both C and ASM programs. There should be an overall
>description of what the chunk of code is doing and there should also be line
>by line comments as necessary to help clarify what each part of the code is
>doing (especially in ASM). For my money, I would always rather have too many
>comments than not enough. Too many "guru's" out there think their code is so
>"self-documenting" that they are blind to the fact that no one else can
>initially understand it without a careful perusal of each line of code.


Written Chinese is also self-documenting. :)
--
Dave's Engineering Page: http://www.dvanhorn.org
Where's dave? http://www.findu.com/cgi-bin/find.cgi?kc6ete-9

--
http://www.piclist.com hint: PICList Posts must start with ONE topic:
[PIC]:,[SX]:,[AVR]: ->uP ONLY! [EE]:,[OT]: ->Other [BUY]:,[AD]: ->Ads


2001\03\08@121457 by Douglas Wood

picon face
Having spent some time in China writing code for a "Hot Box" detector system
for trains that had to display Chinese characters in addition to English,
it's fair to say that NONE of the Chinese that *I* wrote was
"self-documenting"! :-)

Douglas Wood
Software Engineer
.....dbwoodKILLspamspam.....kc.rr.com

Home of the EPICIS Development System for the PIC and SX
http://www.piclist.com/techref/member/DW--RA4

{Original Message removed}

2001\03\08@122156 by David VanHorn

flavicon
face
At 11:19 AM 3/8/01 -0600, Douglas Wood wrote:
>Having spent some time in China writing code for a "Hot Box" detector system
>for trains that had to display Chinese characters in addition to English,
>it's fair to say that NONE of the Chinese that *I* wrote was
>"self-documenting"! :-)

It's self-documenting if you already know what it says.
Something like "cart hot, pu hao!" ?
:)

--
Dave's Engineering Page: http://www.dvanhorn.org
Where's dave? http://www.findu.com/cgi-bin/find.cgi?kc6ete-9

--
http://www.piclist.com hint: PICList Posts must start with ONE topic:
[PIC]:,[SX]:,[AVR]: ->uP ONLY! [EE]:,[OT]: ->Other [BUY]:,[AD]: ->Ads


2001\03\08@123741 by Douglas Wood

picon face
On the other hand, I didn't make any disparaging remarks about anyone's
mother, either!! ;-)

Douglas Wood
Software Engineer
EraseMEdbwoodspam_OUTspamTakeThisOuTkc.rr.com

Home of the EPICIS Development System for the PIC and SX
http://www.piclist.com/techref/member/DW--RA4

{Original Message removed}

2001\03\08@210623 by Olin Lathrop

face picon face
> Wow as a newbie, your code is very understandable. Do you have a site with
> more of that style code.

See http://www.embedinc.com/pic.  There is a lot more code I intend to add
to it eventually.  For now, there is an extensive set of macros I use for
PIC development.  These include bank switching, general registers, a
software stack, subroutine linkage, and some other tidbits.


*****************************************************************
Olin Lathrop, embedded systems consultant in Devens Massachusetts
(978) 772-3129, olinspamspam_OUTembedinc.com, http://www.embedinc.com

--
http://www.piclist.com hint: PICList Posts must start with ONE topic:
[PIC]:,[SX]:,[AVR]: ->uP ONLY! [EE]:,[OT]: ->Other [BUY]:,[AD]: ->Ads


2001\03\08@214543 by Ian Hynes

flavicon
face
PICers,

I'd just like to say a word of thanks to the guys who offered advice
on LCDs. Along with a lot of hot air there's a few gems of REALLY
USEFUL information on this list.

Ian.


myke predko wrote:
>
> Hmmmm....
>
> I guess I should have jumped in a bit earlier - I didn't think the emails
> would get as angry as they did so quickly.
>
>

--
http://www.piclist.com hint: PICList Posts must start with ONE topic:
[PIC]:,[SX]:,[AVR]: ->uP ONLY! [EE]:,[OT]: ->Other [BUY]:,[AD]: ->Ads


2001\03\22@130918 by Bill Westfield

face picon face
{Quote hidden}

This thread died out a long time ago, but as long as I'm picking on Olin, I
thought I'd say that I find this example over-commented (and the comments
over-wordy, too.)  It's not so bad in this rather simple example, but I'd be
very worried that in code that has some algorithmic complexity, that the
"important" comments for the tricky parts would be lost in the general
busy-ness of text.  Even here, we have:

>             skip_ncarr          ;propagate carry to high address byte
>             incf    pclath

Manipulation of PCLATH, perhaps one of the most confusing aspects to a PIC,
and NO COMMENT!?  As a non-experienced PIC programmer, "incf" was a bit
shocking (but I guess I see how it works - pclath has to be "current" to
have gotten to the subroutine...)  Some of the non-obvious macros were a bit
disconcerting as well, but I guess they're OK since they're likely to be
commented to Olin's standards as well...

BillW

--
http://www.piclist.com hint: To leave the PICList
@spam@piclist-unsubscribe-requestKILLspamspammitvma.mit.edu


2001\03\22@155923 by Olin Lathrop

face picon face
> This thread died out a long time ago, but as long as I'm picking on Olin,
I
> thought I'd say that I find this example over-commented (and the comments
> over-wordy, too.)  It's not so bad in this rather simple example, but I'd
be
> very worried that in code that has some algorithmic complexity, that the
> "important" comments for the tricky parts would be lost in the general
> busy-ness of text.

I think this is part of a more general issue of comment scope.  It is just
as important to document the details in "algorithmically complex" code as it
is in any other code.  However, what you really want is comments with
varying scope.  In other words, I want to write a paragraph that applies to
the next 80 lines, but within that maybe three separate paragraphs that each
apply only to a subsection of the 80 lines, etc, etc.

Most languages allow end of line comments and separate lines that contain
only comments.  This easily allows for two levels, but that is often not
enough.  I usually put a line of stars above higher level comments.
Sometimes a long "bar" above very high level comments with shorter bars
above lower level comments, but this is easily lost on those not familiar
with the convention.  Nonetheless, I think it is still better than not
trying at all.  For example:

;
;************************************************************************
;
;   Subroutine DOIT
;
;   High level comment that applies to the whole subroutine.
;
    add   this          ;explain what/why, applies only this line or next few
    stuff that
    blah  counter, w
;
;******************
;
;   The approriate number is in COUNTER.  Now puflbloob the
;   frammistan.
;
    poof  counter
    bark  @tree
    ;
    ;   This comment applies just to the following section until the next
    ;   whole line comment of any type.
    ;

If you do this consistantly others will be able to follow once they get used
to the style.  I still wish there was a way built into the language to
declare the scope of a comment without impeding readability, but there
isn't.

> Even here, we have:
>
> >             skip_ncarr          ;propagate carry to high address byte
> >             incf    pclath
>
> Manipulation of PCLATH, perhaps one of the most confusing aspects to a
PIC,
> and NO COMMENT!?  As a non-experienced PIC programmer, "incf" was a bit
> shocking (but I guess I see how it works - pclath has to be "current" to
> have gotten to the subroutine...)

Now let's be fair, Bill.  If you include the line right above these two, you
will see what I really wrote was:

           addwf   fsr, w      ;make table entry address low byte
           skip_ncarr          ;propagate carry to high address byte
           incf    pclath

This is an add that produces something referred to as a "low byte", followed
by "propagate carry", which in this case takes two instructions.  I don't
think it is the job of comments to explain the opcodes, except perhaps when
unusual side effects are being used.  Comments are better spent describing
the PURPOSE of the opcodes, like doing an add to create the low byte and
then propagating the carry.

> Some of the non-obvious macros were a bit
> disconcerting as well, but I guess they're OK since they're likely to be
> commented to Olin's standards as well...

Yes, they are.  See STD.INS.ASPIC at http://www.embedinc.com.  You can even
use them yourself.


********************************************************************
Olin Lathrop, embedded systems consultant in Littleton Massachusetts
(978) 742-9014, KILLspamolinKILLspamspamembedinc.com, http://www.embedinc.com

--
http://www.piclist.com hint: To leave the PICList
RemoveMEpiclist-unsubscribe-requestTakeThisOuTspammitvma.mit.edu


2001\03\22@192718 by Bill Westfield

face picon face
   I think this is part of a more general issue of comment scope.  It is
   just as important to document the details in "algorithmically complex"
   code as it is in any other code.  However, what you really want is
   comments with varying scope.  In other words, I want to write a
   paragraph that applies to the next 80 lines, but within that maybe three
   separate paragraphs that each apply only to a subsection of the 80 lines

Agreed.


   Most languages allow end of line comments and separate lines that contain
   only comments.  This easily allows for two levels, but that is often not
   enough.

Sometimes this is done with separate source files.

It occurs to me, though, that perhaps the biggest commenting lack I've seen
in "embedded microcontroller" documentation/commenting is a sort of
"bibliography" of the datasheets and other info used to put together the
code and algorithms.  It's GREAT to know something like "as described in
Microchip Applications-note AN1234", or "As per Knuth V3pp134-137" for
instance.



   Now let's be fair, Bill.  If you include the line right above these
   two, you will see what I really wrote was:

               addwf   fsr, w      ;make table entry address low byte
               skip_ncarr          ;propagate carry to high address byte
               incf    pclath

Right, but what was confusing me was that I didn't see how you could be
sure that pclath contained a value that pointed to the current section
of code.  You only set it before explicit page changes, and there are
other ways to get to a new code page, right?  I was assuming that your
"call" macro set pclath in order to call "message", so you could be
confident it was right, but maybe this is just a bug waiting to happen :-)


   I don't think it is the job of comments to explain the opcodes, except
   perhaps when unusual side effects are being used.

Agreed.  This is one of the differences between commenting code for
educational purposes vs documenting "real" code, though.  If you're posting
an example of PIC code as an education in PIC programming, your comments
will look a bit different than they will if you're posting DFT code for
purposes of explaining how to implement a DFT...

BillW

--
http://www.piclist.com hint: To leave the PICList
spamBeGonepiclist-unsubscribe-requestspamBeGonespammitvma.mit.edu


2001\03\23@081204 by Olin Lathrop

face picon face
> Right, but what was confusing me was that I didn't see how you could be
> sure that pclath contained a value that pointed to the current section
> of code.  You only set it before explicit page changes, and there are
> other ways to get to a new code page, right?  I was assuming that your
> "call" macro set pclath in order to call "message", so you could be
> confident it was right, but maybe this is just a bug waiting to happen :-)

I use the convention that PCLATH always points to the current code page.
Yes, this IS documented, but elswhere.  Of course you couldn't have known
that, but I was trying to demonstrate commenting style, and not deeply
explain the code.


********************************************************************
Olin Lathrop, embedded systems consultant in Littleton Massachusetts
(978) 742-9014, TakeThisOuTolinEraseMEspamspam_OUTembedinc.com, http://www.embedinc.com

--
http://www.piclist.com hint: PICList Posts must start with ONE topic:
[PIC]:,[SX]:,[AVR]: ->uP ONLY! [EE]:,[OT]: ->Other [BUY]:,[AD]: ->Ads


2001\03\23@090912 by Bob Ammerman

picon face
> I use the convention that PCLATH always points to the current code page.
> Yes, this IS documented, but elswhere.  Of course you couldn't have known
> that, but I was trying to demonstrate commenting style, and not deeply
> explain the code.

Olin,

I understand how it is easy to ensure that PCLATH matches the current
256-byte code page on entry to a routine and by resetting it after a call,
but how do you handle the case where there is a 256 byte boundary between
the entry point and where you do a computed jump?

Bob Ammerman
RAm Systems
(contract development of high performance, high function, low-level
software)

--
http://www.piclist.com hint: PICList Posts must start with ONE topic:
[PIC]:,[SX]:,[AVR]: ->uP ONLY! [EE]:,[OT]: ->Other [BUY]:,[AD]: ->Ads


2001\03\23@123913 by Olin Lathrop

face picon face
> I understand how it is easy to ensure that PCLATH matches the current
> 256-byte code page on entry to a routine and by resetting it after a call,
> but how do you handle the case where there is a 256 byte boundary between
> the entry point and where you do a computed jump?

You load PCLATH with the high byte of the table start address, which I
forgot to do in the example.  Oops.


********************************************************************
Olin Lathrop, embedded systems consultant in Littleton Massachusetts
(978) 742-9014, RemoveMEolinspamTakeThisOuTembedinc.com, http://www.embedinc.com

--
http://www.piclist.com hint: PICList Posts must start with ONE topic:
[PIC]:,[SX]:,[AVR]: ->uP ONLY! [EE]:,[OT]: ->Other [BUY]:,[AD]: ->Ads


2001\03\23@130411 by Harold M Hallikainen

picon face
On Thu, 22 Mar 2001 16:26:43 PST William Chops Westfield
<billwEraseMEspam.....CISCO.COM> writes:
> It occurs to me, though, that perhaps the biggest commenting lack
> I've seen
> in "embedded microcontroller" documentation/commenting is a sort of
> "bibliography" of the datasheets and other info used to put together
> the
> code and algorithms.  It's GREAT to know something like "as
> described in
> Microchip Applications-note AN1234", or "As per Knuth V3pp134-137"
> for
> instance.
>
>

       I try to do this. It's often the URL of a specific post in the PICLIST
archive...

Harold


FCC Rules Online at http://hallikainen.com/FccRules
Lighting control for theatre and television at http://www.dovesystems.com

________________________________________________________________
GET INTERNET ACCESS FROM JUNO!
Juno offers FREE or PREMIUM Internet access for less!
Join Juno today!  For your FREE software, visit:
dl.http://www.juno.com/get/tagj.

--
http://www.piclist.com hint: PICList Posts must start with ONE topic:
[PIC]:,[SX]:,[AVR]: ->uP ONLY! [EE]:,[OT]: ->Other [BUY]:,[AD]: ->Ads


2001\03\23@131838 by Bob Ammerman

picon face
----- Original Message -----
From: Olin Lathrop <EraseMEolin_piclistspamEMBEDINC.COM>
To: <RemoveMEPICLISTEraseMEspamEraseMEMITVMA.MIT.EDU>
Sent: Friday, March 23, 2001 12:27 PM
Subject: Re: [PIC]: Code Commenting, was: LCD problems revisited


> > I understand how it is easy to ensure that PCLATH matches the current
> > 256-byte code page on entry to a routine and by resetting it after a
call,
> > but how do you handle the case where there is a 256 byte boundary
between
> > the entry point and where you do a computed jump?
>
> You load PCLATH with the high byte of the table start address, which I
> forgot to do in the example.  Oops.

Yeah, I thought it was something like that.

Bob Ammerman
RAm Systems
(contract development of high performance, high function, low-level
software)

--
http://www.piclist.com hint: PICList Posts must start with ONE topic:
[PIC]:,[SX]:,[AVR]: ->uP ONLY! [EE]:,[OT]: ->Other [BUY]:,[AD]: ->Ads


2001\03\23@134409 by jamesnewton

face picon face
One of the best (although sometimes painful) things about this list is the
way that any code you post with a dumb mistake in it is instantly torn
apart... <GRIN> Mostly in a nice way.

Anyone who hasn't read "the Cathedral and the Bazaar" should find the time
to do so...
http://www.tuxedo.org/~esr/writings/cathedral-bazaar/

This IS our tribe.

---
James Newton (PICList Admin #3)
RemoveMEjamesnewtonspam_OUTspamKILLspampiclist.com 1-619-652-0593
PIC/PICList FAQ: http://www.piclist.com or .org

{Original Message removed}

More... (looser matching)
- Last day of these posts
- In 2001 , 2002 only
- Today
- New search...