Searching \ for '[PIC] First Project' 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/microchip/devices.htm?key=pic
Search entire site for: 'First Project'.

Exact match. Not showing close matches.
PICList Thread
'[PIC] First Project'
2005\06\11@234916 by Marcel Birthelmer

flavicon
face
Hi all,
I'm fairly new to the list and I also just "finished" my first (slightly
trivial) project, so I thought I'd share. There's a website about it
here: http://marcel.carrietech.com/pic/spiled.php . It's just a counter
that interfaces with a MAX7221 via SPI to drive some LED displays. It
works pretty well at this point, except for one thing: the two most
significant digits (thousands and hundreds) aren't initialized for some
reason, even though I'm clrf'ing them. They don't fluctuate once the pic
is running, and they behave as expected, but I can't get them to start
at 0 for some reason. If anyone has any ideas, please let me know. Also,
any constructive criticism on code or whatever else is appreciated.
Thanks,
- Marcel

2005\06\12@071851 by Dave W Turner

picon face
Well, I like the site - really nice layout.  The circuit also looks
pretty tidy (except the wires ;-)).  I don't really understand the
code, because I have never worked with software SPI before, but it
looks nice.  In general, it looks a lot better than my second project
- It takes 8 LEDs on portb, puts 1 in port b, and rlf portb.  ;-).

On 6/12/05, Marcel Birthelmer <spam_OUTmarcelTakeThisOuTspamcarrietech.com> wrote:
{Quote hidden}

> -

2005\06\12@080054 by Martin Farrell

flavicon
face
Marcel Birthelmer wrote
>It works pretty well at this point, except for one thing: the two most
>significant digits (thousands and hundreds) aren't initialized for some
>reason, even though I'm clrf'ing them. They don't fluctuate once the
>pic is running, and they behave as expected, but I can't get them to
>start at 0 for some reason. If anyone has any ideas, please let me
>know. Also, any constructive criticism on code or whatever else is
>appreciated.
Hello,

I can see a problem with the code. You've numbered the counters 0x28 to
0x31 rather than 0x28 to 0x2b. Then in the code you inc FSR so the most
significant counters won't be updated when you expect.

That's the sort of bug that's hard to find.
--
Martin

2005\06\12@080341 by olin piclist

face picon face
Marcel Birthelmer wrote:
> Also, any constructive
> criticism on code or whatever else is appreciated.

You used the <name>: construct for code labels.  I wasn't even aware MPASM
allowed this.  In any case, labels should be clearly delineated from the
rest of the code.  The other MPASM label syntax enforces this by requiring
labels to start in column 1 and opcodes in column 2 or further.  This is a
good idea you should follow anway.  Also, it's a good idea to put labels on
a line by themselves.  That makes maintenence easier as inevitably you will
need to add some code right after a label.

I ran your code thru my ASPIC_FIX program to reformat it in a consistent way
that I'm comfortable with.  I had to fix the code labels manually, but I'll
probably add the <name>: syntax to ASPIC_FIX so I don't have to do this in
the future.

>          OUT1    EQU 0x20
>          OUT2    EQU 0x21
>          IN1     EQU 0x22

1 - Comment all your variables.
2 - Comment all your variables.
3 - You really should comment all your variables.
4 - This is a bad way to allocate variables.  First, now that you've had
your fun and gotten the LED blinker out of the way, it's time to do things
right and use relocatable mode.  Do it now before you've settled into the
bad habit of absolute mode.

In relocatable mode, use the RES directive to allocate variables.  Even in
absolute mode, CBLOCK is better for defining variable name symbols (note
that it doesn't actually allocate RAM) than just giving each one an explicit
value.

>          CTR1    EQU 0x28
>          CTR2    EQU 0x29
>          CTR3    EQU 0x30
>          CTR4    EQU 0x31

I just had to laugh at this one.  You are using explicit HEX constant, but
you are counting in decimal.

>          CS      EQU 0
>          MISO    EQU 2
>          MOSI    EQU 1
>          SPICLK  EQU 2

Whatever these are THEY SHOULD BE EXPLAINED in end of line comments at the
very least.

>          org     0
>          goto    init
>          org     0x10

I don't get the point of this.  You don't have an interrupt routine, so
there is no reason your code can't start at 0 and continue directly from
there.  Even if not, what's special about 10h?

> init     clrf    PORTB       ;clear port b

Again, I think it's bad form to put executable instructions on the same line
as a label.

>          bsf     STATUS, RP0
>          bcf     TRISA, SPICLK

This sort of manual bank switching is dangerous.  I think you got it right
in this case, but sooner or later you'll make a mistake looking up the bank
of an SFR in the data sheet or setting the RPx bits accordingly.  For
starter programs that are not space or speed cricital, just use BANKSEL.
Once you get more comfort with PICs you'll probably want to use something
like my DBANKIF and related macros in STD.INS.ASPIC at
http://www.embedinc.com/pic.

This particular case is particularly bad since you are assuming that RP1 is
0 but there is no mention of that restriction.  Some day you'll insert code
above here and unrelated stuff will break.

>          clrf    PORTB

Why?  You already cleared it a few instructions earlier.

>          bsf     PORTB, CS

Ah, so CS stands for a particular bit number in port B.  You should SAY SO.
Also check out my /INBIT and /OUTBIT preprocessor directives.  They handle
I/O port bit definitions in a nicer way.

>          call    xfer16
>
>          movlw   0x0A        ;INADDRSITY : MAX
>          movwf   OUT1

This is a serious bug because you are relying on XFER16 to either preserve
the bank or set it to 0, but XFER16 is not defined to do either.  (Actually
none of its actions are defined, but that's another problem.)

Just because you are getting away with it here doesn't make this any less of
a bug.

> mainloop
>          movlw   CTR1        ;load starting address
>          movwf   FSR
>
> bump     incf    INDF, 1     ;increase current digit

Indirect bank setting?

{Quote hidden}

Finally a few comments about the details, but all higher level explaination
is completely missing.  You may not think of it as a bug, but it certainly
is, and a rather serious one at that.  There should be block comments in
front of important labels like MAINLOOP and BUMP.  When do we get back here?
What gets done each time thru the loop?  When does the loop terminate?

> bump_done
>          clrf    ADDR

Again, you are assuming the loop above terminates with bank 0 set, but I
don't see anything specifying that, so it can't be relied on.  There should
also be a comment here that the bank setting is assumed to be 0.

> xfer16   bcf     PORTB, CS   ;assert /CS

>From previous context, this is apparently a subroutine.  So WHERE ARE THE
COMMENTS!!!!?  Beginner or not, this sort of programming is simply
inexcusible and irresponsible.  What is the purpose of this subroutine?
What does it take as input?  What state does it change?  What state does it
trash?

There should be something in the code that makes it visually easy to see a
major break in flow.  I use a comment line with all stars.  A proper
subroutine comment header should make the break in the code visually
obvious, and it should tell a programmer everything he needs to know to call
the subroutine.  You should be able to give other programmers just the
subroutine documentation header and they should be able to call the
subroutine correctly and deal with all its side effects without needing the
source code.


*****************************************************************
Embed Inc, embedded system specialists in Littleton Massachusetts
(978) 742-9014, http://www.embedinc.com

2005\06\12@132312 by Marcel Birthelmer

flavicon
face
Olin Lathrop wrote:
> Marcel Birthelmer wrote:
>
>>Also, any constructive
>>criticism on code or whatever else is appreciated.
>
>
> You used the <name>: construct for code labels.  I wasn't even aware MPASM
> allowed this.  In any case, labels should be clearly delineated from the
> rest of the code.  The other MPASM label syntax enforces this by requiring
> labels to start in column 1 and opcodes in column 2 or further.  This is a
> good idea you should follow anway.  Also, it's a good idea to put labels on
> a line by themselves.  That makes maintenence easier as inevitably you will
> need to add some code right after a label.

I'm using gpasm to assemble, and the labels are actually in the first
column, it's just that my php script on the website decided to mangle
this and I didn't catch it. And I think the : after the label is just
ignored.

{Quote hidden}

So you think I should comment my variables?

> 4 - This is a bad way to allocate variables.  First, now that you've had
> your fun and gotten the LED blinker out of the way, it's time to do things
> right and use relocatable mode.  Do it now before you've settled into the
> bad habit of absolute mode.
I didn't know about this, but I will definitely research.

{Quote hidden}

And that would be the initialization bug. Thanks to you and the other
person who pointed this out.

{Quote hidden}

Yeah at one point I thought I was going to have an interrupt routine,
and since I wasn't being very space-conscious with this code, so I
spaced it out a little bit, and then I didn't bother reorganizing it.
It's awkward, yes, but not really harmful.

{Quote hidden}

OK, I can see how relying on the bank to stay the same can be dangerous,
but considering that except for accessing the TRIS* registers, I'm not
moving beyond the first bank at all in this program, I don't think it's
a "serious bug".

{Quote hidden}

OK, that's probably fair. But on the other hand, you wouldn't do a full
doxygen-compatible documentation on "Hello World", either, so I think as
long as I'm just screwing around and changing things every two seconds
anyway, it's not a case of criminal negligence. Your point is definitely
noted, though, and I promise the next time I submit something to the
list it will be soaked with comments and documentation.

Thanks very much for your in-depth review and also for pointing out the
silly bug that was causing me trouble thus far.

- Marcel

2005\06\12@133535 by Peter Onion

flavicon
face
On Sun, 2005-06-12 at 10:25 -0700, Marcel Birthelmer wrote:

> I'm using gpasm to assemble

Olin said ....
> > it's time to do things
> > right and use relocatable mode.  Do it now before you've settled into the
> > bad habit of absolute mode.


> I didn't know about this, but I will definitely research.

Just to say gpasm DOES support relocatable code, and Olin is right, make
the move now and save yourself the pain later !

Peter

2005\06\12@134222 by olin piclist

face picon face
Marcel Birthelmer wrote:
> I'm using gpasm to assemble

It would be good to point out you are using a non-standard assembler
whenever you post code.  Otherwise people will rightfully assume the code it
intended for MPASM.

> Yeah at one point I thought I was going to have an interrupt routine,
> and since I wasn't being very space-conscious with this code, so I
> spaced it out a little bit, and then I didn't bother reorganizing it.
> It's awkward, yes, but not really harmful.

This is another thing automatically handled in relocatable mode.  The fixed
code at the reset vector jumps to relocatable code, which the linker puts
wherever it fits best.

> OK, I can see how relying on the bank to stay the same can be
> dangerous,
> but considering that except for accessing the TRIS* registers, I'm not
> moving beyond the first bank at all in this program, I don't think
> it's
> a "serious bug".

It will be soon enough as you start writing anything beyond a simple LED
blinker.  If you don't religiously consider banking, you're going to waste a
lot of time not so far down the road.


*****************************************************************
Embed Inc, embedded system specialists in Littleton Massachusetts
(978) 742-9014, http://www.embedinc.com

2005\06\12@150833 by Peter Onion

flavicon
face
On Sun, 2005-06-12 at 13:42 -0400, Olin Lathrop wrote:
> Marcel Birthelmer wrote:
> > I'm using gpasm to assemble
>
> It would be good to point out you are using a non-standard assembler
> whenever you post code.  Otherwise people will rightfully assume the code it
> intended for MPASM.

Ok, I'm going to stick my neck out a bit here, but I believe gpasm's
developers aim is to make it be as compatible as possible with MPASM.
I've only ever used gpasm and I've never come across any code on the web
that gpasm won't assemble, and while gpasm may have features missing I
don't think it has any extras that would make valid gpasm code fail to
work on MPASM.

But I could be wrong.

Peter

2005\06\12@151625 by olin piclist

face picon face
Peter Onion wrote:
> Ok, I'm going to stick my neck out a bit here, but I believe gpasm's
> developers aim is to make it be as compatible as possible with MPASM.
> I've only ever used gpasm and I've never come across any code on the
> web
> that gpasm won't assemble, and while gpasm may have features missing I
> don't think it has any extras that would make valid gpasm code fail to
> work on MPASM.

Does MPASM support the <name>: code label syntax?  I hadn't seen that before
in MPASM source.  For a long time, Microchip had a bunch of secret features
in MPASM that wasn't in the MPASM manual.  I think they were listed in the
MPLAB help, but of course you'd have to know to ask to get them.  Is this
possibly one of them, or is it an outright incompatibility in GPASM?


*****************************************************************
Embed Inc, embedded system specialists in Littleton Massachusetts
(978) 742-9014, http://www.embedinc.com

2005\06\12@153359 by Peter Onion

flavicon
face
On Sun, 2005-06-12 at 15:16 -0400, Olin Lathrop wrote:

> Does MPASM support the <name>: code label syntax?  I hadn't seen that before
> in MPASM source.

You'll have to try it yourself Olin, as I don't have a windows machine
here at all !

Peter



2005\06\12@161639 by Wouter van Ooijen

face picon face
> Ok, I'm going to stick my neck out a bit here, but I believe gpasm's
> developers aim is to make it be as compatible as possible with MPASM.
> I've only ever used gpasm and I've never come across any code
> on the web that gpasm won't assemble

My WISP firmware? But I admit I used a lot of undocumented features.

Wouter van Ooijen

-- -------------------------------------------
Van Ooijen Technische Informatica: http://www.voti.nl
consultancy, development, PICmicro products
docent Hogeschool van Utrecht: http://www.voti.nl/hvu


2005\06\12@171356 by Jinx

face picon face
> noted, though, and I promise the next time I submit something to the
> list it will be soaked with comments and documentation.

As a favour, would you (and everyone, as James often asks) also
mind trimming your post to NOT include the entire message you're
replying to. Thanks

> as long as I'm just screwing around and changing things every two
> seconds anyway, it's not a case of criminal negligence

Not for you maybe, as you're working on this code and familiar with
it, but someone coming in cold AND looking for a bug needs help to
get up to speed

2005\06\12@173839 by William Chops Westfield

face picon face
On Jun 12, 2005, at 5:00 AM, Martin Farrell wrote:

> You've numbered the counters 0x28 to 0x31 rather than 0x28 to 0x2b.

Heh.  I did that myself recently.  A 5bit mask of ones : 0x31, right?

Hmmph.
BillW

2005\06\12@175527 by Dave W Turner

picon face
> Does MPASM support the <name>: code label syntax?

Yep, sure does.  I have copy-pasted lots of code with label: bla bla
in, and it compiled fine without a twitch.  I personally think this is
quite nice to do, because the colon makes it a lot clearer that it's a
label, and not just some instruction that no-one has heard of.  I
often get confused with what's labels when reading piclist emails -
gmail does a rather nice job of screwing up tabs and indents.

>  Heh.  I did that myself recently.  A 5bit mask of ones : 0x31, right?

Huh????  Yeh, i mean that 31 is a row of 5 ones, but how on earth is
that relevent?  Don't mean to flame, but it is a bit confusing.

Donkey.

Pencil sharpener.

7*6=42.

--

Dave
All us base are belong to you.

2005\06\12@180149 by William Chops Westfield

face picon face

On Jun 12, 2005, at 10:25 AM, Marcel Birthelmer wrote:

>>> Also, any constructive
>>> criticism on code or whatever else is appreciated.
>> [Olin's comments]
> [counter-arguments]
>
The best code reviews ARE discussions; you need not do everything
everyone
suggests, but you should have reasons in your own mind to dismiss their
comments.  I think this has been a good example...

BillW

2005\06\12@181303 by William Chops Westfield

face picon face

On Jun 12, 2005, at 2:55 PM, .....dave.w.turnerKILLspamspam@spam@gmail.com wrote:

>>  Heh.  I did that myself recently.  A 5bit mask of ones : 0x31, right?
>
> Huh????  Yeh, i mean that 31 is a row of 5 ones, but how on earth is
> that relevent?

It's the same error as the original poster's program - thinking in
decimal and writing in hex.  31 is 5 ones.  0x31 is something else!

 :-)
BillW

2005\06\12@183323 by Dave W Turner
picon face
Oh, e.g.

0x30 - 0x29 = 0x5
whereas:
d'30' - d'29' = d'1'

Well, you won't want to see my code then - I am constantly switching
between decimal, hex and binary - binary for config registers, hex for
things like delays which aren't specified exactly, and need to be
fiddled with, decimal for exact numbers over 9.  Somehow I manage....
Must be the number base conversion on my calculator ;-).

On 6/12/05, William Chops Westfield <westfwspamKILLspammac.com> wrote:
{Quote hidden}

> -

2005\06\12@201752 by Dave Tweed

face
flavicon
face
Dave <EraseMEdave.w.turnerspam_OUTspamTakeThisOuTgmail.com> wrote:
> Oh, e.g.
>
> 0x30 - 0x29 = 0x5

????

Better let the assembler handle your arithmetic,
rather than doing it in your head.

-- Dave Tweed

2005\06\12@223940 by Bob Ammerman

picon face
> Does MPASM support the <name>: code label syntax?  I hadn't seen that
> before
> in MPASM source.

Yes, it does, as do most assemblers.

I use it because then I can find the label easily in a text editor (instead
of seeing all the references to the label).

I also put the label on a line by itself for four reasons:

1: it can be long without messing up the code format
2: I can easily add code immediately after it
3: It gives me a place for comments to describe, for example, a loop
invariant
4: I just like the way it looks ;-)

Bob Ammerman
RAm Systems

2005\06\13@044517 by Alan B. Pearce

face picon face
>> I just had to laugh at this one.  You are using explicit
>> HEX constant, but you are counting in decimal.
>
>And that would be the initialization bug. Thanks to you and
>the other person who pointed this out.

It is also the sort of bug that gets ruled out by using the RES directive,
because you would define it as

   CTR RES 4    ; 4 byte counter - 1 byte per digit

and then in your code refer to them as CTR, CTR+1, CTR+2, CTR+3 which also
helps to show the relationship of the bytes, rather than as 4 separate
variables, which could be independent counters for other uses.

2005\06\13@073238 by olin piclist

face picon face
Bob Ammerman wrote:
> I use it because then I can find the label easily in a text editor
> (instead of seeing all the references to the label).

You can do the same thing with the other label format since those are forced
to start in column 1.

> I also put the label on a line by itself for four reasons:
>
> 1: it can be long without messing up the code format
> 2: I can easily add code immediately after it
> 3: It gives me a place for comments to describe, for example, a loop
> invariant
> 4: I just like the way it looks ;-)

I agree completely.


*****************************************************************
Embed Inc, embedded system specialists in Littleton Massachusetts
(978) 742-9014, http://www.embedinc.com

2005\06\14@202619 by Jinx

face picon face
> I also put the label on a line by itself for four reasons:

Another reason (unless there's an error turn-off) is that a macro
name can't share a line with a label

eg, Illegal label (WAIT_SSP), build fails

wait_ssp   banksel pir1

but assembles OK as

wait_ssp   ;comment
          banksel pir1

2005\06\15@070419 by olin piclist

face picon face
Jinx wrote:
> Another reason (unless there's an error turn-off) is that a macro
> name can't share a line with a label

I frequently put my UNBANK macro on the same line as a label.

*****************************************************************
Embed Inc, embedded system specialists in Littleton Massachusetts
(978) 742-9014, http://www.embedinc.com

2005\06\15@070946 by Jinx

face picon face

> > Another reason (unless there's an error turn-off) is that a macro
> > name can't share a line with a label
>
> I frequently put my UNBANK macro on the same line as a label.

How do you stop MPLAB/MPASM failing the build ?

2005\06\15@074223 by Bob Ammerman

picon face
It isn't macros that can't be labeled, rather it is some of the standard
pseudo-ops in the assember.

Bob Ammerman
RAm Systems

{Original Message removed}

2005\06\15@075655 by Jinx

face picon face

> It isn't macros that can't be labeled, rather it is some of the standard
> pseudo-ops in the assember.

Yes, that's true now I look more closely at what fails in a build. banksel
for instance. My own macros don't cause a problem (working very long
hours on the same project and had a bit of brain fade)

What would be handy (jeepers, hope I get it right this time) is being
able to "Set PC At Cursor" on a macro. Also, bookmarks saved
when file/project closed. Breakpoints are

2005\06\15@082619 by olin piclist

face picon face
Jinx wrote:
>> I frequently put my UNBANK macro on the same line as a label.
>
> How do you stop MPLAB/MPASM failing the build ?

I don't do anything special.  It just works.  Before your post I had never
considered this an issue (and still don't really believe it is).


*****************************************************************
Embed Inc, embedded system specialists in Littleton Massachusetts
(978) 742-9014, http://www.embedinc.com

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