Jump to content

Orin

EstablishedMember
  • Content Count

    55
  • Joined

  • Last visited

Everything posted by Orin

  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.
  15. This option is only available for PIC18. One has to ask "Why?". I could save at least 10 instructions in one of my interrupt routines if I could do it myself... No array indexing or pointers, so no need to save FSR. PCLATH is never used in the program so no need to save or set it. clrf status instead of two bit clears. This was significant when I had a budget of 120 instructions to handle the interrupt. Orin.
  16. You might be better off doing something like: something = ((shift_reg>>3) ^ shift_reg) & 0x01; I do similar in one of my projects: if ( (((RXProtocolByte >> 1) ^ TXProtocolByte) & (1<<SEQBIT)) == 0 ) Try them all and see what gives shorter code... Orin.
  17. If I REALLY wanted to save those 9 words, I'd do it with inline assembly... asm { movlw 0xFF movwf _Speeds movwf _Speeds+1 ; and so on up to _Speeds+9 } Orin. I'm not that desperate to save a few bytes..yet. It's just that I come from a Windows C++ engineering background where I blink and consume more than 2k bytes, so this whole embedded world is pretty different! That's my 'real' job too. Personally, I consider the PIC compiler a tool to provide the structure of the program and to do the tricky things - like handling multi-byte arithmetic/compares/status flags. To use it effectively, I look at the code the compiler is producing in order to get the most efficient code. Some things are going to be expensive whatever - non-constant array indexes and pointer operations for example. Something as simple as changing for loops that will be executed at least once to do { } while (); loops will save code. Count down and terminate on zero will allow a future compiler optimization that will save more code unless you are indexing an array off your loop variable... then consider reversing the order of the array elements! Orin.
  18. If I REALLY wanted to save those 9 words, I'd do it with inline assembly... asm { movlw 0xFF movwf _Speeds movwf _Speeds+1 ; and so on up to _Speeds+9 } Orin.
  19. Fred, Are you absolutely sure you are interrupting on the falling edge of the signal? Looking at your first diagram, if you were interrupting on the RISING edge, then the type 2 and 3 intervals would indeed be swapped. Orin.
  20. Well, lines 1 and 3 look OK apart from the first value... which tends to indicate a problem with the initial values of your variables when portb.0 goes low for the first time. At this point, is the timer on? What would the value of IRstreambit be at this point? Orin.
  21. I think you should clear TMR1IF immediately before setting TMR1ON. There is a small window of opportunity between testing INTF in interrupt() and calling StopTmr1() in IR_ISR_handler() where the timer could overflow and set TMR1IF. I suspect under normal circumstances, it couldn't happen (as in your MPLAB simulation) but a noisy IR signal could cause it to happen. Orin.
  22. I don't think you need to do anything with INTE during IR_ISR_handler unless it is intentional to return without setting it at the end of the third if. Do you reset TMR1IF before enabling the timer in your StartTmr1 function? Orin.
  23. Which PIC? For most, you cannot take a second interrupt while handling the first unless you were to deliberately set the global interrupt enable flag in the interrupt routine and what a can of worms that would be... Or are you talking about a routine that runs as a result of an interrupt, but not in the interrupt context? In that case, it would be perfectly reasonable to disable both interrupt enables during the routine. Orin.
  24. What do you mean by 'starting' or 'stopping' interrupts? There shouldn't be any need to do anything as the interrupt routine itself can't be interrupted. I usually use something of the form: void interrupt() { nextInterrupt: if ( xxIF ) { xxIF = 0; // Handle xx goto nextInterrupt; } if ( yyIF ) { yyIF = 0; // Handle yy goto nextInterrupt; } } It implements a priority scheme where interrupt xx takes priority over interrupt yy. If you want to disable an interrupt temporarily, then set the IE bit to zero. Change the tests to if ( xxIF && xxIE ) if you don't want to handle an interrupt that you have 'disabled'. Orin.
×
×
  • Create New...