Looking for reviews of this rookies first attempt at coding

[CudaBlu]

I am looking for pointers on the attached code. I feel I have learned a lot by researching this forum and some of you will surely see code snippets of yours here. But for better and I'm afraid worse I will take responsibility for this code in its entirety. Am looking for areas to improve, other coding techniques that could be used and would like to know if there is a slicker way to handle the if...then section of code at the bottom.

Thank you to everyone who contributes to this forum for getting me this far.

#PICAXE18m2

symbol VSL  = 2						        ;Variable Start Location (ie, b2)
symbol VEL  = VSL + 6					        ;Variable End Location (# of variables)
symbol VLOS = 30					        ;Variable Location OffSet amount
symbol LSP  = VSL + VLOS				        ;Loop Start Position
symbol LEP  = VEL + VLOS				        ;Loop End Position

symbol Sc   = b2						        ;Seconds from DS1307
symbol Mn   = b3						        ;Minutes from DS1307
symbol Hr   = b4						        ;Hours from DS1307
symbol DOW  = b5					        ;Day of week from DS1307
symbol DD   = b6						        ;Date from DS1307
symbol MM   = b7						        ;Month from DS1307
symbol YY   = b8						        ;Year from DS1307

hi2csetup i2cmaster,%11010000,i2cslow_8,i2cbyte	;Set PICAXE as Master with DS1307 as Slave at address

hi2cin 0,(b2,b3,b4,b5,b6,b7,b8)			        ;Read time & date into variables (BCD format)

for bptr = LSP to LEP
  lookup b0,(Sc,Mn,Hr,DOW,DD,MM,YY),b1		;Loop through time & date values
  b1 = b1 / 16 * $FFFA + b1				        ;Convert BCD to ASCII
  bptr = bptr - VLOS					        ;Calculate RAM location
  poke bptr, b1						        ;Poke variable value to RAM location
  bptr = bptr + VLOS					        ;Reset RAM location to continue loop
  inc b0							        ;Increment lookup value
next bptr

if        DOW = 1 then sertxd ("Sun")		        ;Convert Day of Week number (1-7) to text
  elseif DOW = 2 then sertxd ("Mon")		        ;Convert Day of Week number (1-7) to text	
  elseif DOW = 3 then sertxd ("Tue")		        ;Convert Day of Week number (1-7) to text
  elseif DOW = 4 then sertxd ("Wed")		        ;Convert Day of Week number (1-7) to text
  elseif DOW = 5 then sertxd ("Thu")		        ;Convert Day of Week number (1-7) to text
  elseif DOW = 6 then sertxd ("Fri")		                ;Convert Day of Week number (1-7) to text
  elseif DOW = 7 then sertxd ("Sat")		                ;Convert Day of Week number (1-7) to text
endif

sertxd (#Hr,":",#Mn,":",#Sc,cr,#MM,"/",#DD,"/",#YY) ;Write out string to screen

pause 7000

This code just reads the date and time values from a DS1307, converts the values, and then displays them with sertxd

 

[BeanieBots]

A well structured presented and above all, commented peice of code.
Well done.

Nothing wrong with your If/Elsif structure.
As with most code listings there will always be alternatives but none would be particular better, just different.
Personally, I prefer to use a select/case structure for such conversions.
Others might prefer to use a lookup table.

Where I think you might gain some speed and possibly reduce space is in the BCD conversion but I'll leave it to others who are better at such code tricks than myself to comment further.

 

[hippy]

You could do the day of week reporting with a SELECT-CASE and there's also this option but no idea how it compares for speed or size. Can remove the leading "-" if you reduce DOW by one, use 0-6 rather than 1-7 -

Lookup DOW, ("-SMTWTFS"), b0 : SerTxd( b0 )
Lookup DOW, ("-uouehra"), b0 : SerTxd( b0 )
Lookup DOW, ("-nneduit"), b0 : SerTxd( b0 )

 

[westaust55]

I concur that the program is well laid out and well docuemnted.

As a couple of comemnts:
1. I would recommend against changing the value of the index variable within a FOR...NEXT loop.
It may work but is not best practice.

2. Where you have the line:

POKE bptr, b1​

you can use:

@bptr = b1 ; save the value in byte variable b1 at the location pointed to by the RAM byte pointer (bptr)​

 

[CudaBlu]

Thank you to all who replied, your comments have been received and acted upon.

@BeanieBots & hippy

The change from the if...then to LookUp for the day of the week saved 47 bytes of memory (wow)! I just discovered that you can run the syntax check and it gives you the memory used, I'll start paying attention to that for now on.

@westaust55

I changed the bptr to @bptr. In the manual (being new at this) I couldn't understand what the difference was so I just used what worked. Thanks for the input.