Jump to content

Orin

EstablishedMember
  • Content Count

    55
  • Joined

  • Last visited

Community Reputation

0 Neutral

About Orin

  • Rank
    Regular
  1. I have done some test with a spreadsheet. Round off error is indeed a problem. Scaling the values up by a couple of powers of 2 may solve this problem. Most of the math is 16 bit and I would like it to work well for 10 bit a to d. More info when I look into it some more. The division is by 8 not 7 so the shift is fine, but for the loss of bits off the right hand end. I almost suggested this method in my original reply. It's a filter of the form: Filtered value = (New value * p) + ((1-p)*Filtered Value) where 0 < p < 1. The suggested filter has p = 1/8. It scales the right hand side by 8 to get: Filtered value * 8 = New value + (7 * Filtered Value) As for roundoff errors, aren't we talking about a truncation error if the divide is done by a simple shift? I'd add 4 before the shift to round rather than truncate the result. Note that 7 * Filtered value might be better calculated as ((Filtered value << 3) - Filtered Value). It might generate less code than ((Filtered value <<2) + (Filtered value <<1) + Filtered Value) for a multiply by 7. Yes, this filter does still have a significant contribution from the 8th previous sample, as would picking p = 1/4. I'd be tempted to use p = 1/2 which makes the filter: Filtered value = (Filtered value + New value + 1) / 2 which is much easier to calculate. Orin.
  2. I wouldn't recompute the total each time. I'd do the following: /* Zero nTotMagX and xMagArray at program initialization */ nTotMagX -= aMagXArray[nPos]; /* remove oldest value from sum */ aMagXArray[nPos] = SensorMagX; /* save new value */ nTotMagX += SensorMagX; /* add new value to sum */ nPos = (nPos + 1) & 7; nMagXAveragedValue = nTotMagX / 8; This does assume you have enough RAM for the extra global variables, nTotMagX (and nTotMagY). If you wanted to use a pointer instead of nPos to step through the array: /* Program initialization */ int *pOldestMagX = &aMagXArray[0]; //... nTotMagX -= *pOldestMagX; /* remove oldest value from sum */ *pOldestMagX = SensorMagX; /* save new value */ nTotMagX += SensorMagX; /* add new value to sum */ if ( ++pOldestMagX == &aMagXArray[8] ) { pOldestMagX = &aMagXArray[0]; } I don't know which would be better. You'd just have to look at what the compiler produced in each case. Orin.
  3. MS VC6 lets me do: typedef volatile unsigned char vuchar; vuchar c = 1; without complaint, so I don't see why not. Orin.
  4. If you want an infinite loop, you should use for ( ; ; ) { }. while ( 1 ) { } will generate warnings with some compilers. FWIW, the sematics of the for loop are as follows: for ( expression1; expression2; expression3 ) statement means: #ifdef SOME_VERSIONS_OF_C_PLUSPLUS { #endif expression1; while ( expression2 ) { statement; expression3; } #ifdef SOME_VERSIONS_OF_C_PLUSPLUS } #endif If expression2 is empty, it's taken to be true. The surrounding braces are due to the scope of variables declared in "expression1" (it's not just an expression in C++). For example in, for ( int i = 0; i < something; i++ ) { }, i is in scope only within the loop in some versions of C++. I don't know what scope Sourceboost gives i in this case. Orin.
  5. There is nothing wrong with doing all your work in an interrupt routine and have your main routine simply sleep waiting for an interrupt if there is no 'background' processing to do. For this particular code: sleep() should be in an infinite loop. You shouldn't touch GIE in the interrupt routine as it's reset by the interrupt logic and set by the return from the interrupt routine. Orin. On further thought, there is a major problem with the original code: sleep() will disable the clock to the timer, so the timer interrupt will never occur while asleep. So, you should not sleep if the timer is enabled. Note that you can't simply do: if ( !t1con.TMR1ON ) sleep(); as there is a possibility of taking an interrupt between the test and the sleep. The following is probably safe for the main loop: for (;; ) { intcon.GIE = 0; /* Disable interrupts */ if ( !t1con.TMR1ON ) { sleep(); /* An interrupt will wake the CPU up even with GIE clear */ } intcon.GIE = 1; /* Enable interrupts. If we slept and an interrupt is pending, we will take the interrupt now. */ } Orin.
  6. There is nothing wrong with doing all your work in an interrupt routine and have your main routine simply sleep waiting for an interrupt if there is no 'background' processing to do. For this particular code: sleep() should be in an infinite loop. You shouldn't touch GIE in the interrupt routine as it's reset by the interrupt logic and set by the return from the interrupt routine. Orin.
  7. You are quite right. We just started looking at this problem and it happens exactly as you described. We'll check if it can get fixed. Even though sum was promoted by compiler the resulting code should work. Can you please point where you think wrong code was generated. Regards, Pavel Hi Pavel This is the bad code , result will always end up set to NO_FRUIT // TWO #defined constants if(result >= (APPLES + PEARS)) 0004 0E13 MOVLW 0x13 0006 84D8 BSF STATUS,Z 0008 6001 CPFSLT gbl_result 000A B4D8 BTFSC STATUS,Z 000C D001 BRA label1 000E D002 BRA label2 0010 label1 0014 label2 { result = NO_FRUIT; 0010 0E80 MOVLW 0x80 0012 6E01 MOVWF gbl_result } It looks like the compiler is assuming that CPFSLT sets Z. CPFSLT doesn't affect any status bits. Orin.
  8. I agree. What the ap note fails to mention is that for the method to work, the power line neutral would need connecting to the PIC's ground line. Expect that someone somewhere will get the hot and neutral lines crossed - the circuit ground is now at 120V or worse...
  9. Works fine if you remove the inline from "inline void GetValueInline(char& c) { setTo12©; }" so the problem is passing a reference to a structure to an inline function. Note that neither the address of a or the address of b are passed in any case. Call by reference is implemented by copying the variable to the function parameter, then copying the function parameter back to the variable when the function returns. Indeed, in the 'inline' case, it is msg.a that is copied/restored rather than msg.b. Orin.
  10. Try the following: char shadowA; shadowA = porta; shadowA.0 = 0; if ( packet.0 ) shadowA.0 = 1; porta = shadowA; Or even: asm { movf _porta, W // repeat per bit andlw 0xFE btfsc _packet, 0 iorlw 0x01 movwf _porta // } BTW, all bets are off if the variable 'packet' isn't in the same bank as porta as the compiler will insert bank switching code. Yes, the assembly code is a slow read - modify - write of porta. Remember bsf and bcf on porta are implemented in the PIC hardware as read-modify-write so there's no real difference. Orin.
  11. I couldn't agree more. In addition, to do what was suggested would penalise those who require the fastest overall execution speed. Not to mention for a non volatile destination, there's nothing wrong with the compiler implementing the assignment as: dest = 0; if ( src.0 ) dest = 1; // and so on You could spend hours tinkering with the C to get the compiler to produce the code you want and the timings you want only to have the next version of the compiler do something slightly differently and break your code. So, if you want precise timings, you absolutely have to use assembly. No big deal, the compiler supports inline assembly. Orin.
  12. Orin

    Xoring Bug?

    It would appear that a^b^c^e^f^g is compiling to something like ( a^b ) |c|e|f|g. Orin.
  13. Heh. I was just about to post solution! For me this now operates at 12 cycles (while the @ing is now 9 for some reason), so it reduces it by a deascent amount! Assuming the B value is greater than the A value is fine. That's not a restriction or anything. In fact this: if ( porta.PORTA_BIT_A ^ porta.PORTA_BIT_B ) { portb = 1; } Performs as well as the @ operator... I must have mis-read my code when I analysed it before: if ( ra1 ^ ra3 ) { 0018 3000 MOVLW 0x00 0019 1985 BTFSC main_1_ra3,3 001A 3001 MOVLW 0x01 001B 1885 BTFSC main_1_ra1,1 001C 3A01 XORLW 0x01 001D 39FF ANDLW 0xFF 001E 1903 BTFSC STATUS,Z 001F 2823 GOTO label2 And: if ( porta.PORTA_BIT_A ^ porta.PORTA_BIT_B ) { 0036 3000 MOVLW 0x00 0037 1985 BTFSC gbl_porta,3 0038 3001 MOVLW 0x01 0039 1885 BTFSC gbl_porta,1 003A 3A01 XORLW 0x01 003B 39FF ANDLW 0xFF 003C 1903 BTFSC STATUS,Z 003D 2840 GOTO label4 This is great. I always thought that operator was not a good way of accessing bits. And seeing as the bits I'm accessing are constant for the duration of execution this is perfect. Thanks again for your input, Orin. Much appreciated. I was just about to post a similar comment. Yes, for constant bits, "porta.PORTA_BIT_A ^ porta.PORTA_BIT_B" seems to be best. (Future compiler optimization? If the compiler uses instructions that set the zero flag at addresses 0036 and 0038, (eg: ANDLW 0x00 and IORLW 0x01), it could omit the ANDLW at address 003B.) I couldn't access bits this way as I was accessing a structure member and the compiler doesn't accept structure.member.bit (yet?). Orin.
  14. Actually, I wasn't commenting on whether your proposal worked or not, just adding another alternative. Try: if ( ((porta>>(PORTA_BIT_B-PORTA_BIT_A)) ^ porta) & (0x01<<PORTA_BIT_A) ) { portb = 1; } It shifts porta such that the bits of interest overlap, xors them together and tests the overlapping bit. It does assume that PORTA_BIT_B > PORTA_BIT_A... It only does two shifts rather than 4, a saving of 4 ticks for the shifts alone. It might avoid a second temporary variable and load/store for additional savings. This method will only work for bits that are close together BTW or you spend more time shifting than the alternatives would take. Orin.
×
×
  • Create New...