Jump to content

davidb

EstablishedMember
  • Content Count

    134
  • Joined

  • Last visited

Everything posted by davidb

  1. Keni, Sorry I must have mis-understood your post as I assumed from it that you were trying to drive an LCD display and LEDs directly over a 30 metre cable using the PIC logic level outputs. These are unsuitable for driving transmission lines so at the very least I would expect them to be connected to some form of line driver. I would be interested to see how you are driving the LCD and LEDs with DC. Perhaps I have the wrong end of the stick again. Now, thats what you call interference! Cheers davidb
  2. Al. If you are going to check an interrupt flag in a main() function rather than in interrupt() then DO NOT enable the interrupt. The interrupt flag gets set regardless of the state of its enable. Also don't forget to clear the flag when read. BTW Using an interrupt to wake from sleep with a button press might be your only option. Regards davidb
  3. Keni, 30m!!! I am surprised it worked at all. I wouldn't dare use standard logic levels over more than 300mm. If that is what you are doing I would consider a re-think before you run into other problems. Regards davidb
  4. Hi Tim, The interrupt template I gave was just an example for Al. I didn't expect him or anybody else to copy it literally, whether they used the particular interrupts shown or not. I just find it useful when starting a new project to remind me of the various interrupt enables, control bits, flags etc. available for a particular micro without having to look them up. Normally the individual interrupt checks are commented out until I need to use them. You will also notice that the interrupt enable checks do not have to be used and can be easily commented out as required. In addition the interrupts can be moved around to change the priority. The reason that the enable check is sometimes required is that the interrupt might be temporarily disabled within the code for operational reasons. Since the interrupt flag can still be set the check is there to prevent the interrupt being processed incorrectly when other interrupts are processed. If the interrupt is never disabled then the check can be removed. Obviously in your Timer 0 example where only one interrupt is used you could get away with not checking the enable or the flag. I never have that situation but I think I would still at least check the flag! As far as using inline interrupts or if...else structure, it entirely depends on what you are trying to do. If you have many interrupts that could occur at a similar time then the if...else structure would probably slow things down since you might exit the isr then immediately re-enter it to process the next interrupt with the consequent extra overhead. On the other hand it does save time by not having to check any lower priority enables and flags. I really do not know which is the best method to use and I think some discussion on this backed up by a few tests might be interesting. Regards davidb
  5. Sorry Ian I agree with Reynard and Al. A variable is only initialised to zero if it is declared static AND the -Su switch is not invoked. All other variables are left uninitialised. Regards davidb
  6. Al. It depends on what you want to happen. I do not have any code readily available but here are my thoughts: From power up run through basic initialisation and then place it in sleep mode waiting for a button press (using RB0 int or int on change etc.) i.e. off mode. A single button press will then wake the micro from sleep, run through a bit more initialisation and then start up your application by running main(). In your main() periodically do a simple check for a button press. You will need to define a time period in which the 3 successive button presses must be detected. The first press would start a timer/counter as well as a button press count and then continue executing the main loop. The overall timing depends on how fast your main loop executes. You could simply use another counter if the main loop period is consistent. If 3 button presses and releases are detected within the chosen time period then reset the micro using either reset() or a loop and watchdog timeout. This will take you back to the off mode (sleep). Less than 3 presses and releases within the chosen period will reset the timer and button press counter and carry on running the application. You will probably need some basic de-bouncing for the button on and off. If you need your application to freeze in main() rather than re-initialise when turned off then that is a different problem. Regards davidb
  7. Al. Yes, you are correct that the literal GPIE will always be true - must drink stronger coffee in the morning! Having said that why did you put it in as a condition? The second part of my answer is correct if you want to check that the interrupt is enabled AND that the interrupt has occured, otherwise simply check the interrupt flag. Regards davidb
  8. Alistair, GPIE is defined as 7 and GPIF as 0 therefore (GPIE && intcon.GPIF) can never be true! Change to if(intcon.GPIE && intcon.GPIF) or just if(intcon.GPIF) and all should be well providing there are no other problems. Don't forget to clear the interrupt flag as well. Regards davidb
  9. Alistair, Were you asking a question or simply making a statement? Anyway, your code is still faulty for the same reason you showed and therefore will not work as you think! If you are not sure why then look at the code generated. I always use a standard template for every interrupt for each device I use and simply change the order of the interrupts or comment them out as required. That way I don't have to re-invent the wheel for each project. The comments help me to remember what else I need to check/set. The code can be cleaned up later once working if required but I don't normally bother. The following is a small example from the template I use for an 18F4520. I haven't bothered to show interrupt_low() but this is usually a repeat of interupt(). void interrupt(void) { // External Interrupt on RB0 - Always High Priority. Check Pull-ups intcon2,RBPU & Edge intcon2,INTEDG0 if(test_bit(intcon,INT0IE) && // Interrupt enabled check (Comment out if not required) test_bit(intcon,INT0IF)) { // Place code or function call here for External Interrrupt 0 Handler clear_bit(intcon,INT0IF); // Clear Interrupt Flag } // External Interrupt on RB2 - Check Priority intcon3,INT2IP. Check Pull-ups intcon2,RBPU & Edge intcon2,INTEDG2 if(test_bit(intcon3,INT2IE) && // Interrupt enabled check (Comment out if not required) test_bit(intcon3,INT2IF)) { // Place code or function call here for External Interrupt 2 Handler clear_bit(intcon3,INT2IF); // Clear Interrupt Flag } // Timer 0 if(test_bit(intcon,TMR0IE) && // Interrupt enabled check (Rem out if not required) test_bit(intcon,TMR0IF)) // Timer 0 Interrupt? { // Place code or function call here for Timer 0 Interrupt Handler clear_bit(intcon,T0IF); // Clear interrupt flag } // Compare Capture Module 2 if(test_bit(pie2,CCP2IE) && // Interrupt enabled check (Rem out if not required) test_bit(pir2,CCP2IF)) // Compare Module 1 Interrupt? { // Place code or function call here for Compare/Capture Module 2 Interrupt Handler clear_bit(pir2,CCP2IF); // Clear interrupt flag } // Repeat format for all other interrupts... } Hope that helps Regards davidb
  10. Is this what you are trying to do? asm { movlw 0x0000111 movwf _option_reg } The above example is pointless really but if you want to use asm then use lower case register names and preceed the name with an underscore. For multiple line asm surround the asm lines with braces on their own lines. Regards davidb Just realised also that 0x0000111 should be written 0b00000111 asm { movlw 0b00000111 movwf _option_reg } Regards davidb
  11. Is this what you are trying to do? asm { movlw 0x0000111 movwf _option_reg } The above example is pointless really but if you want to use asm then use lower case register names and preceed the name with an underscore. For multiple line asm surround the asm lines with braces on their own lines. Regards davidb
  12. You do need to read the adc value within the loop as Kenn said. However this alone will not solve the problem. At the moment you only read the adcvalue once before the loop and then repeatedly add that value 16 times followed by dividing it by 16 after the loop. The next time around you do exactly the same so that your average increments by the new adcvalue each time round you main loop. This is obviously not what you intended. You could as you suggest initialise the average before the loop. It really depends on what you want to do but you need to consider the sampling rate, number of samples, how many samples you want to average over, moving averages etc. I would suggest you look up averaging filters. As a start I would suggest you look at http://www.piclist.com/techref/microchip/math/filter.htm. The code is asm but should give you a few ideas. Regards davidb
  13. Shree, your average is being constantly incremented by adcvalue! Regards davidb
  14. Hi, What is the actual problem? This seems to work for me: #pragma DATA _EEPROM, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff, 0xff Unless it was a typo the OP missed the comma after _EEPROM Cheers davidb
  15. Pierre, When I first started PIC programming in asm I forced myself to do everything the preferred Microchip way and obviously did such a good job I completely forgot about the 'old' non-preferred way of using tris! As far as I can see you have correctly disabled the comparators and your code does appear to toggle RA4 on the simulator. Without trying it on real hardware myself I can only suggest that you check your hardware: 1. Check that you haven't connected the LED and resistor to the wrong port pin. Been there and done that. 2. If you haven't already done it connect the LED to one of the other outputs and check that it toggles. 3. Use a meter with the PIC power off to confirm that the open drain transistor on RA4 is not permanently shorted. 4. Try another PIC. BTW what resistor are you using to limit the LED current? One in the range 180R to 680R should be OK. Regards davidb
  16. Pierre, First take a look at your configuration code: I am not sure what you intended your code to do but if you want to set the direction of a port or port pin you must write a value to the appropriate TRIS register e.g. I haven't shown it but you also need to manipulate the bank bits as the TRIS registers are in bank 1. Don't forget to change back again. Regards davidb
  17. Pierre, Yes you are doing something wrong! Your logic is inverted. With the LED tied to Vdd if ra4 is set i.e. Logic 1 then your LED will be off. Clear ra4 i.e. Logic 0 and it will be on. Regards davidb
  18. Hi Henry, Correct, It doesn't normally work for SFR's as they are normally defined as char that is why I suggested a re-defined shadow of eeaddr as a short. I agree, we have probably wasted enough time on this particular subject. Regards davidb
  19. Hi Henry, NO, of course it doesn't update two registers at once since the PIC18F is only an 8-bit device. This is the main reason we use high level language; it mostly hides the low level operations required by the processor to carry out what we require, regardless of the processors native data size. If you write a short to a short as above, regardless of whether it is an FSR that has been re-defined as a short or a normal memory location the compiler converts this into writing both lo and hi bytes. Try looking at the code generated by the compiler e.g. the above would be: MOVF main_1_address, W MOVWF main_1_eeadr_16bit MOVF main_1_address+D'1', W MOVWF main_1_eeadr_16bit+D'1' The fact that the two registers are not actually written to simultaneously can sometimes be a problem, particularly with timers A-D etc but that's another story. Regarding your other comments Pavel has mostly covered these. Regards davidb
  20. Henry, I think we are getting bogged down by confusion in the detail, probably because I didn't explain myself very well. The overall discussion was about how to overcome the fact that the latest version of the eeprom lib does not work for PIC18F parts with only 256 byte eeprom. This is because not all PIC18F parts have the hardware register EEADRH aka eeadrh. eeadrh is used in the function for ALL PIC18F variants so any program using the library for a target with only 256 byte eeprom fails to link. The discussion now is how to recognise the two types of PIC18F parts and generate a common library file. Apart from putting conditionals in for each part the only thing we have to recognise the difference is the definition EEADRH. I might be wrong but I don't think this can be used when generating the eeprom lib file. Regarding the discussion on char and shorts it wasn't about how the current function was meant to work in C, that is understood, but more about how to modify it. I was thinking at register level to take account of both 8 and 16 bit addresses. Regardless of whether it was a good idea or not I suggested that you could define eeadr or a shadow of it as a short in the header files for all parts that have 16 bit addressing for the eeprom. e.g. in PIC18F8723 you could define a version of eeadr as follows (eeadr itself can be left defined as a char): volatile short eeadr_16bit @EEADR; This therefore defines eeadr_16bit as eeadr & eeadr+1. For consecutive register addressing eeadr+1 happens to be the same as EEADRH aka eeadrh. Therefore if you use eeadr_16bit in place of eeadr in the function then eeadr and eeadrh will both be updated if a short is written to it. So in place of: // Load address eeadr = address; #ifdef EEADRH eeadrh = address >> 8; #endif we could simply use: // Load address eeadr_16bit = address; With devices that only have 256 byte eeprom i.e. no EEADRH writing a char to eeadr_16bit will still update it correctly as will a short which will simply lose the top byte. This only works if the address eeadr+1 is unused which may or may not be the case. The upshot of this is that eeadrh would no longer be needed in the function so if it was defined or not it wouldn't matter. The main problem is that a shed full of header files would have to be modified and it is also non standard. I haven't tried this but I do not see why it wouldn't work anyway please ignore my ramblings! Pavel is making proposals on how to overcome the problem for the next version. I agree with his latest thinking of having separate eeprom lib files for the two eeprom variants of PIC18F. I tried modifying the eeprom source files in the way he suggested previously and when compiled with a project they appeared to work for all variants but generating a common library from these is a different matter. Regards davidb
  21. Pavel, I missed your last post with the improved header to take account of EEADRH so ignore my last post on request for information. For my own sanity I have modified local copies of eeprom.h & eeprom.c and at least proved that it will compile for targets with different eeprom sizes. Regards davidb
  22. Pavel, You say there is no side effect to using a PIC18F with only 256 byte eeprom but it no longer links for a PIC18F4520 with SB V6.96 whereas it used to on the RC version. The following simple example shows the problem: #include <system.h> #include <eeprom.h> // Target PIC18F4520 void main() { unsigned char temp = 0x0f; unsigned char addr = 0xff; while(1) { eeprom_write(addr, temp); temp = eeprom_read(addr); } } This compiles OK but produces the following when trying to link: Notice that it failed because eeadrh could not be resolved because it is not defined in PIC18F4520.h I resolved the problem in my project by not using the library but could you let us know what you have changed in eeprom.c to resolve this matter with eeadrh. Regards davidb
  23. Henry, The facts: 1. eeadr (eeadrl) is always defined as a char in all the PIC header files that have eeprom of any size. 2. eeadrh is only defined as a char in PIC header files that have more than 256 bytes of eeprom. 3. The current build of the eeprom lib will not work with PIC18Fs with only 256 byte eeprom because eeadrh is NOT defined for these parts. 4. The curent eeprom.c and eeprom.h need to be modified in some way - The clunky way would be to put a conditional in eeprom.c to only use eeadrh for every PIC18F that has only >256 byte eeprom (or the inverse of this). This would be long winded and would probably need regular updating as Microchip launch even more parts. Correct me if I am wrong, but what you seem to be implying is that if eeadr & eeadrh are in consecutive hardware registers in lo/hi byte order (as they probably are) then writing a short to just eeadr will also write the high byte of the short to eeadrh. Surely, this cannot be correct as shorts should always be converted (truncated). Writing a short to any char will only write the lo byte to the char and the top byte will be lost. However, if you defined eeadr (or a mirror of it to avoid confusion) as a short then writing a short to just eeadr should update both eeadr and eeadrh correctly. If you are correct in your assumptions then I am going back to asm! Regards davidb
  24. Henry, All I was trying to say was that I don't think Pavels proposal can work without some re-write as currently eeadr is defined as a char in the PIC18F header files. This will work With a PIC16F or a PIC18F with 256 bytes of eeprom but with a PIC18F with >256 bytes the short will be converted to a char i.e. eeadrh will not be updated. To make it work both eeprom.c and eeprom.h and possibly the appropriate PIC18F header files need to be re-written. Perhaps I am missing something simple. Regards davidb
  25. Pavel, I see where you are going with function overloading but I think you still have a problem and need to determine when eeadrh is required otherwise how do you determine when to use a short or char? As it stands eeprom.c needs re-writing. If PIC18F address is always a short then this will be converted to char when you write to eeadr. It might simplify matters if you could define eeadr or probably a different variable name (to avoid conflicts) as a short (EEADR + EEADRH) wherever EEADRH exists. This would need to be done in each PIC18F header file. I haven't checked all the PIC18F parts but I assume that where EEADRH is used it is contiguous and in the right order with EEADR! Regards davidb
×
×
  • Create New...