-
How to combine TMR0H + TMR0L into a 16 bit variable using XC8
Posted by
DTJ
on 10 Feb, 2016 09:05
-
Hoping for some help with a basic PIC XC8 question:
I'm counting pulses using TMR0, converting the resulting count to ASCII & displaying it on an LCD.
To get the value in TMR0 into the counter variable I do it the long way as shown below, it works ok.
Q) I imagine I can do it much simpler. How to I get it to just concatenate the two TMR0 registers?
Thanks.
unsigned int counts = 0; //counts variable
counts = TMR0H*256+TMR0L; // load current TMR0 counter value
itoa(buffer,counts,10); // convert counts to ASCII string & place them in buffer array
// now write buffer to LCD
-
-
counts = (TMR0H << 8) | TMR0L;
unsigned int *ptr = &count + 3;
*ptr = TMR0L; ptr--;
*ptr = TMR0H;
// it depends on the endianess
-
#2 Reply
Posted by
helius
on 10 Feb, 2016 09:16
-
C does not have any concatenate operator. The standard library has string catenating functions, but they do not work on binary values.
Either use counts = (TMR0H<<8 ) ^TMR0L;
or union {unsigned v; struct {unsigned char h, l;} s;} counts;
counts.s.h = TMR0H; counts.s.l = TMR0L;
itoa(buffer, counts.v, 10);
-
#3 Reply
Posted by
Ian.M
on 10 Feb, 2016 09:43
-
If you are using Timer 0 in 16 bit mode, you must be using a PIC18 as all other families supported by XC8 have an 8 bit Timer 0.
The PIC18 Timer 0 has a latch for the high byte, which is used both for read and write operations. TMR0H is the latch, not the actual timer. To write the timer you must first write TMR0H to load the latch then write TMR0L to transfer the latch to the timer high byte, while simultaneously writing the low byte. To read the timer you must first read TMR0L which also updates the latch from the high byte of the timer, then read TMR0H to get the previously latched value.
Although the XC8 headers include a definition for a 16 bit TMR0, It should NOT be used as you are relying on
undefined behaviour, namely the order of byte reads or writes to a 16 bit volatile unsigned short.
Even your:
counts = TMR0H*256+TMR0L; // load current TMR0 counter valueis unsafe as the compiler is free to rearrange operations between sequence points and the RHS doesn't contain any.
To guarantee a correct read, even if you upgrade the compiler or change optimisation level, you *must* use:
counts = TMR0L;
counts|=TMR0H*256U; // load current TMR0 counter value
or a similar sequence to ensure you get the high byte that was latched most recently.
-
#4 Reply
Posted by
DTJ
on 10 Feb, 2016 11:40
-
Many thanks for the replies gentlemen. Based on Ian's comments ( & my lack of reading the data sheet), I was also using the incorrect method to write to the Timer.
I've found the ReadTimer0() and WriteTimer0() functions also. Is it preferable to use these? I imagine they should take into account subtleties you have pointed out.
I also noted the 'U' suffix on the 256 in:
counts|=TMR0H*256U; // load current TMR0 counter value
Is this necessary or just good practice? I did some reading and it seems to indicate that the constant (256) is unsigned. I thought that would be implicit anyway.
Also I imagine using the 8 left shifts would be faster than multiplying by 256 - is that correct?
Thanks for your help guys, really appreciated.
-
#5 Reply
Posted by
Xenoamor
on 10 Feb, 2016 11:43
-
union {unsigned v; struct {unsigned char h, l;} s;} counts;
counts.s.h = TMR0H; counts.s.l = TMR0L;
itoa(buffer, counts.v, 10);
+1
-
#6 Reply
Posted by
dannyf
on 10 Feb, 2016 11:58
-
The fastest approach would be to use a 16-bit pointer and point to TMR0L - in cases where TMR0H and TMR0L are consecutive. This approach suffers from the same flaw as the struct approach in that they are endian-dependent.
In general, it is safer to take a look at the datasheet and see precautions on operating 16-bit types on an 8-bit mcu -> atomocity isn't always assured if yo don't follow the right sequence of operations.
-
#7 Reply
Posted by
Ian.M
on 10 Feb, 2016 12:49
-
@Helius, Xenoamor: You are wrong. TMR0L *MUST* be read first. See any PIC18 datasheet. Even if you fix it, the type-punning union approach is also generally regarded as bad practice, and is non-portable.
@DannyF: Its an 8 bit CPU with only three index registers, each split into two parts. Pointers are *SLOW*.
@DTJ,
Its good practice to use unsigned constants when doing unsigned maths. Under ANSI C's usual arithmetic conversions (C89 #3.2.1.5) the unsigned type of TMR0H dominates so in this case it doesn't really matter. Efficiency relies on the compiler recognising the idiom as a byte shift so simply using MOV instructions, which it will also do for unsigned shifts that are multiples of 8 bits, but some non-ANSI compliant PIC compilers (including Microchip C18) don't do standard integer promotion for chars so the result of TMR0H<<8 will be 0.
The functions are actually macros defined in pic18.h so can be used without a performance penalty. As the compiler writers know the current 16 bit byte read order, they will always be correct for the current compiler version and may be faster than the alternative explicit method. However they are non-portable.
-
#8 Reply
Posted by
dannyf
on 10 Feb, 2016 16:07
-
I fed the four approaches (1=using a struct, 2=using a pointer, 3=left shift, 4=multiplication) through XC8 and here is the output:
33: myTMR0_struct->W+=1; //increment the word
1FB2 4AD6 INFSNZ 0xfd6, F, ACCESS
1FB4 2AD7 INCF 0xfd7, F, ACCESS
34: tmp = myTMR0_struct->W; //the struct approach
1FB6 CFD6 MOVFF 0xfd6, 0x1
1FB8 F001 NOP
1FBA CFD7 MOVFF 0xfd7, 0x2
1FBC F002 NOP
35: tmp = myTMR0_ptr; //the pointer approach
1FBE C003 MOVFF 0x3, 0xfd9
1FC0 FFD9 NOP
1FC2 C004 MOVFF 0x4, 0xfda
1FC4 FFDA NOP
1FC6 CFDE MOVFF 0xfde, 0x1
1FC8 F001 NOP
1FCA CFDD MOVFF 0xfdd, 0x2
1FCC F002 NOP
36: tmp = myTMR0_shift; //the left-shifting approach
1FCE 50D6 MOVF 0xfd6, W, ACCESS
1FD0 CFD7 MOVFF 0xfd7, 0x5
1FD2 F005 NOP
1FD4 6A06 CLRF 0x6, ACCESS
1FD6 C005 MOVFF 0x5, 0x6
1FD8 F006 NOP
1FDA 6A05 CLRF 0x5, ACCESS
1FDC 1005 IORWF 0x5, W, ACCESS
1FDE 6E01 MOVWF 0x1, ACCESS
1FE0 5006 MOVF 0x6, W, ACCESS
1FE2 6E02 MOVWF 0x2, ACCESS
37: tmp = myTMR0_mlt; //the multiplication approach
1FE4 50D6 MOVF 0xfd6, W, ACCESS
1FE6 CFD7 MOVFF 0xfd7, 0x5
1FE8 F005 NOP
1FEA 6A06 CLRF 0x6, ACCESS
1FEC C005 MOVFF 0x5, 0x6
1FEE F006 NOP
1FF0 6A05 CLRF 0x5, ACCESS
1FF2 1005 IORWF 0x5, W, ACCESS
1FF4 6E01 MOVWF 0x1, ACCESS
1FF6 5006 MOVF 0x6, W, ACCESS
1FF8 6E02 MOVWF 0x2, ACCESS
Compiler in free mode.
The compiler is pretty smart, isn't it? Especially being free,
-
-
pic18f458.h
// Register: TMR0
extern volatile unsigned short TMR0 @ 0xFD6;
main.c
unsigned short counts = 0;
counts = TMR0;
18f458.as
;main.c: 12: counts = TMR0;
movff (c:4054),(c:_counts) ;volatile
movff (c:4054+1),(c:_counts+1) ;volatile
-
#10 Reply
Posted by
Ian.M
on 10 Feb, 2016 16:36
-
That's what the ReadTimer0() macro generates, a direct 16 bit read of TMR0. If they ever reorder it, the macro will be updated to match.
-
-
18f458.as
;main.c: 12: counts = TMR0;
movff (c:4054),(c:_counts) ;volatile
movff (c:4054+1),(c:_counts+1) ;volatile
you should look how the TMR0 get assigned then...
-
#12 Reply
Posted by
dannyf
on 10 Feb, 2016 16:55
-
That's what the ReadTimer0() macro generates, a direct 16 bit read of TMR0. If they ever reorder it, the macro will be updated to match.
Have you tried to tell Micorchip engineers that those macros they wrote violated your directive?
Although the XC8 headers include a definition for a 16 bit TMR0, It should NOT be used ...
-
#13 Reply
Posted by
Ian.M
on 10 Feb, 2016 17:15
-
Because the macros are provided by the compiler development team, they can make sure that the access order for each macro is correct for the current compiler version. e.g. the WRITETIMER0() macro is currently quite complex because of this:
#define WRITETIMER0(x) ((void)(TMR0H=((x)>>8),TMR0L=((x)&0xFF)))If you use TMR0 directly and a new compiler version breaks your code, that's your problem.
A fatal mistake would be to try to alter the period of the timer in its ISR by:
TMR0+=CyclesToSkipthe same way you would in 8 bit mode using TMR0L.
Its therefore best to avoid using the 16 bit TMR0 definition directly in your code, and if you need to 'bump' the timer like that, use:
{
unsigned t;
t=READTIMER0();
t+=CyclesToSkip;
WRITETIMER0(t);
}
n.b.
WRITETIMER0(READTIMER0()+CyclesToSkip);is also a disastrous mistake, because WRITETIMER0(x) evaluates x twice so would read the timer twice, with the possibility of TMR0L rollover between the two reads.
-
#14 Reply
Posted by
Brutte
on 10 Feb, 2016 17:38
-
main.c: 12: counts = TMR0;I'd also add:
assert(no IRQs accessing TMR0L are enabled);just before that.
-
-
If you use TMR0 directly and a new compiler version breaks your code, that's your problem.
Wonderful. You write programs in C because it's portable and less error-prone, but spend half your time dealing with compiler issues. Then when you try to move the program to another chip you discover what 'portability'
really means...
-
#16 Reply
Posted by
Ian.M
on 10 Feb, 2016 18:26
-
*DON'T* remind me. Code written in CCS C using its idiosyncratic library functions is a bear to port to any other compiler. As soon as you make heavy use of non-ANSI libraries, you've locked yourself in to that compiler vendor. At least if you hit the registers directly, the differences in register bit access syntax can usually be accommodated by simple search and replace.
Porting up and down within one PIC range (e.g. PIC18) isn't usually so troublesome as long as the new device has the peripherals you need and they support the operating modes you have actually used.
-
#17 Reply
Posted by
dannyf
on 10 Feb, 2016 19:53
-
Because the macros are provided by the compiler development team, they can make sure that the access order for each macro is correct for the current compiler version.
Yeah right.
e.g. the WRITETIMER0() macro is currently quite complex because of this:
Or maybe not.
-
#18 Reply
Posted by
hamster_nz
on 10 Feb, 2016 22:06
-
unsigned int counts = 0; //counts variable
counts = TMR0H*256+TMR0L; // load current TMR0 counter value
itoa(buffer,counts,10); // convert counts to ASCII string & place them in buffer array
// now write buffer to LCD
Isn't this code borked anyway?
There is a race condition in that TMR0H may change between when it is read and TMR0L is read, causing the count to be off by 256.
If TMR0H is read first, when the timer is 00:FF and then TMR0L is read when it is 01:00 then you end up with 00:00.
or If TMR0L is read first, when the timer is 00:FF and then TMR0H is read when it is 01:00 then you end up with 01:FF.
Read the high byte first, then the low byte, then the high byte again and check that it hasn't changed. If it has, then rinse and repeat.
unsigned short this_high, low, last_high;
this_high = TMR0H;
do {
last_high = this_high;
low = TMR0H;
this_high = TMR0H;
} while (last_high != this_high);
count = (this_high << 8) | low;
-
#19 Reply
Posted by
mikerj
on 10 Feb, 2016 22:22
-
That's what the ReadTimer0() macro generates, a direct 16 bit read of TMR0. If they ever reorder it, the macro will be updated to match.
Have you tried to tell Micorchip engineers that those macros they wrote violated your directive?
Although the XC8 headers include a definition for a 16 bit TMR0, It should NOT be used ...
You are trying to be a too clever, and failed. There is a 16 bit TMR0 definition which is a literal 16 bit pointer to the TMR0 registers, so write ordering in this case is effectively undefined and could change with compiler revisions (though unlikely). This is was Ian was referring to.
There is also a pair of macros that explicitly define the order of the reads and writes.
-
#20 Reply
Posted by
dannyf
on 10 Feb, 2016 22:55
-
Read the high byte first, then the low byte, then the high byte again and check that it hasn't changed. If it has, then rinse and repeat.
Only if the high byte is unbuffered. It is not necessary in this case as the high byte is buffered.
-
-
or If TMR0L is read first, when the timer is 00:FF and then TMR0H is read when it is 01:00 then you end up with 01:FF.
No. TMR0H is not the timer high byte, it is a buffer. When you read TMR0L it copies the timer high byte to TMR0H. That is why you
must read TMR0L first! (if you read TMR0H first it will have whatever it had on the previous write to TMR0H
or read of TMR0 - whichever was the last operation). Similarly you should write to TMR0H first, then writing to TMR0L updates both the high and low bytes together.
Note: not all PICs have this feature!
-
#22 Reply
Posted by
dannyf
on 10 Feb, 2016 23:26
-
Also your code can be simplified:
do {
msb = TMR0H;
lsb = TMR0L;
} while (msb == TMR0H);
tmr0=(msb<<8) | lsb;
-
#23 Reply
Posted by
westfw
on 12 Feb, 2016 02:15
-
Although the XC8 headers include a definition for a 16 bit TMR0, It should NOT be used as you are relying on undefined behaviour, namely the order of byte reads or writes to a 16 bit volatile unsigned short.
Presumably the combination of the (Microchip-provided) compiler and the (Microchip-provided) access definition combine to provide "defined" behavior that you can complain about if it breaks. Perhaps not very potable to other compilers, though, which is one of the reasons to use a HLL in the first place.
The compiler is pretty smart, isn't it?
Um. WTF are those NOPs doing there, besides doubling the size of the code?
avr-gcc is disappointingly not smart enough to realize that it doesn't need to zero the low byte in "a = a<<8 + PORTB", nor does it recognize that it could INPUT the byte directly to the low register of (16bit) a. Sigh. Although it otherwise produces much better code for the shift case.
-
#24 Reply
Posted by
Ian.M
on 12 Feb, 2016 04:22
-
Although the XC8 headers include a definition for a 16 bit TMR0, It should NOT be used as you are relying on undefined behaviour, namely the order of byte reads or writes to a 16 bit volatile unsigned short.
Presumably the combination of the (Microchip-provided) compiler and the (Microchip-provided) access definition combine to provide "defined" behavior that you can complain about if it breaks. Perhaps not very potable to other compilers, though, which is one of the reasons to use a HLL in the first place.
If you use the Microchip provided macros you get implementation defined behaviour - namely correctly ordered access to timers in 16 bit latched mode - that you can complain about if they break it.
If you use TMR0 (or the other 16 bit Timer word access SFR variables while the timer is in latched mode) directly, currently reads work but writes fail with the previous value of the latch being used. You may not even notice if you are reloading it in an ISR with a constant, but I hnow of a few users doing more complex things with the timers who have already wasted days or weeks debugging it. It doesn't help that the hardware debuggers use code running on the PIC so also interact with the latch, and the simulator has historically had fairly broken and untrustworthy timer handling.