View unanswered posts | View active topics It is currently Sat Jul 22, 2017 7:36 pm



Reply to topic  [ 29 posts ]  Go to page Previous  1, 2, 3  Next
My Digidash project 
Author Message
LQFP112 - Up with the play
User avatar

Joined: Thu Sep 10, 2009 9:23 am
Posts: 244
Location: Dayton, OH
at first glance, this is an awful lot of code to be putting in an isr for a PIC ... the 'wait' statements are especially troubling. imnsho code should never be spinning on anything in an isr.


Mon Dec 07, 2009 4:55 pm
Profile
LQFP112 - Up with the play
User avatar

Joined: Tue Jan 15, 2008 5:31 pm
Posts: 166
I am curious to know why you think this code shouldn't be run in an interrupt handler? The "wait" isn't an actual instruction code, it's an indication to put a few idle cycles to allow the signals to propagate in the external IC's, the PIC will be running at 40 MHz and the A6832 has output latency of about 1µs, plus the logic IC's some of the signals are connected to have between 7 and 20ns of propagation time. Also, I do want to run this on an interrupt, since the LED manufacturer datasheet implied a range of microseconds when dealing with overdriven LED timing, which can be well overshoot if polling for the timer overrun in the main code (I actually want four RTC cycles per display refresh, after doing some correct maths...) It's quite terrain incognito for me, so I'll gladly hear any advice on how and why rather than opinions. Should I modify the program stack in the interrupt handler so that upon exiting, the code would go on at display subroutine, and after that continue where the interrupt happened, or just start back at the beginning?


Mon Dec 07, 2009 8:43 pm
Profile
LQFP112 - Up with the play
User avatar

Joined: Thu Sep 10, 2009 9:23 am
Posts: 244
Location: Dayton, OH
I have nothing other than experience which sets off the red flags .. we'll talk again once you post some real life code then. You're right, I can't comment on how/why until I see some failing code ... but then why did you post your code in the first place other than for comment ? at least thats what I thought. I also assumed that this was from an isr since the pseudocode is framed by "Display interrup handler" - "Return from interrupt". Resetting the watchdog from an interrupt isn't a good idea either .. your manline code could be totally fubar'd, but the interrupt method is completely self sustaining independent of anything else occuring. In my typical code, I setup a series of 'gates' which are set from various points in my code (isr's, mainline, etc) ... once all the 'gates' are open do I then touch the watchdog. it all depends on how much robustness you want - the watchdog (to me) isn't something to dread - if you're going to use it, then make it a crutial part of your architecture and depend on it.

I still stand by original statement. Sorry man, I thought you were posting up looking for comments ;) I'll hold off anything further until we have something tangible to talk about. This is some cool stuff and I get a bit excited.


Mon Dec 07, 2009 10:29 pm
Profile
LQFP112 - Up with the play
User avatar

Joined: Tue Jan 15, 2008 5:31 pm
Posts: 166
EssEss wrote:
I have nothing other than experience which sets off the red flags .. we'll talk again once you post some real life code then. You're right, I can't comment on how/why until I see some failing code ... but then why did you post your code in the first place other than for comment ?

I am and I did, but a vague single liner got me none the wiser, and "IMNSHO" is plain offensive. I didn't think the pseudocode was so far off the intended assembler code it wasn't predictable though.
Quote:
at least thats what I thought. I also assumed that this was from an isr since the pseudocode is framed by "Display interrup handler" - "Return from interrupt".

Correct and correct. I do want comments on this, but something I can work with. Also, the code indeed is in the isr. The biggest delay isn't with the waits or the loops; the SPI interface itself works at Fosc/4, and at worst will cause two 3µs delays (except in the case of ASCII write where it causes one 3µs and one 12µs delay but that's slightly different). The essential feature is the rapid execution of the code.
Quote:
Resetting the watchdog from an interrupt isn't a good idea either .. your manline code could be totally fubar'd, but the interrupt method is completely self sustaining independent of anything else occuring.

In my typical code, I setup a series of 'gates' which are set from various points in my code (isr's, mainline, etc) ... once all the 'gates' are open do I then touch the watchdog. it all depends on how much robustness you want - the watchdog (to me) isn't something to dread - if you're going to use it, then make it a crutial part of your architecture and depend on it.

A good point, and a good way to save an instruction from the display routine. However, since the interrupt timer would be maxed to the overdriven LED timing (~120µs), it would be reasonable to have a watchdog set to much lower time than interrupt period. The PIC would reset way before next display write, or the first if it comes to that. It's not the watchdog I dread, but frying the LEDs. More aggressive the WDT the quicker the lights will go out in a problem situation.
Quote:
I still stand by original statement. Sorry man, I thought you were posting up looking for comments ;) I'll hold off anything further until we have something tangible to talk about. This is some cool stuff and I get a bit excited.


Well, that's nice to hear. But I would still like to know how to make the display routine triggering better.

One thing I did figure out though from the no spinning comment. There's no need for that many loops. The address is always four bytes, and always read from the same intermediary registers. Reading them in-line alone saves eight cycles. using a byte to tell data length or ASCII will save a few commands too. Data for regular LED's still needs to be looped, but the since the data for ASCII is always 8x2 bytes, it may be read and fed in-line too, saving 16 instructions.


Tue Dec 08, 2009 1:55 am
Profile
LQFP112 - Up with the play
User avatar

Joined: Thu Sep 10, 2009 9:23 am
Posts: 244
Location: Dayton, OH
Quote:
A good point, and a good way to save an instruction from the display routine. However, since the interrupt timer would be maxed to the overdriven LED timing (~120µs), it would be reasonable to have a watchdog set to much lower time than interrupt period. The PIC would reset way before next display write, or the first if it comes to that. It's not the watchdog I dread, but frying the LEDs. More aggressive the WDT the quicker the lights will go out in a problem situation.

I didn't mean to totally axe it from the isr, just use other gating methods as a total application wide solution instead of puting all of your eggs in one basket and touching the watchdog from the isr only (which appears to be self sustaining)

I should have been clearer than using 'spinning' - what I mean by that is code which sits looping on a bit (or some other thing like locks/flags) waiting for it to change state. That should never be done from an isr. The act of spinning is the act of waiting, so I automatically thought of non-deterministic looping when I read the word 'wait' in your pseudocode. I ended up reading way too much into it all.


Tue Dec 08, 2009 2:27 am
Profile
LQFP112 - Up with the play
User avatar

Joined: Tue Jan 15, 2008 5:31 pm
Posts: 166
Alright, did some more research, first my assumption that commands run at oscillator pace was plain wrong - they run at 1/4 of the oscillator frequency. Funny how you never pay attention to these things during the class and other time-insensitive applications (Our PIC project at the college was a stepper controller using PIC16f84, not very high-tech), the other was the shortest watchdog seems to be so incredibly slow I indeed really can't use it within the ISR. Somehow I had the idea it was about 1000 times faster than it really was :oops:

Now, because each instruction takes 100ns and not 25, it means the execution of the code is slower, but also I figured out that arranging the tasks so that while the external circuitry is taking their time I can do some other stuff in the code at the mean time and make the routine much more efficient. I also decided to drop the idea of writing the whole ASCII display content in one go. The code would be too cumbersome, the circuit is much too slow to input all digits and control words in one go, and with a digit at a time and the process is much more flexible. Here's a go in attempt to write the actual display code in PIC assembly language.

Code:

;******************************************************************************
;Display interrupt routine for DIGIDASH project.

      ORG   0x0008
      

;*** housekeeping ***;      
      movff   STATUS,STATUS_TEMP      ;save STATUS register
      movff   WREG,WREG_TEMP         ;save working register
      movff   BSR,BSR_TEMP         ;save BSR register

;*** Prepare the A6832 to receive new data ***;
      movff    DDRIVER,PORTD         ;writes the value in DDRIVER (0x01) to PortD, set the 4-to-16 decoder to pull down A6832 latch pin

;*** Write LED address data into the A6832 ***;
      movf   ADDR4,w               ;moves the last address byte from an intermediary register into W
      call   SPIMPolPut            ;function that writes the contents of WReg into SPI
      
      movf   ADDR3,w               ;moves the third address byte into WReg
      call   SPIMPolIsTransmitOver   ;function that waits until the SPI bus is clear
      call   SPIMPolPut            ;feeds the bits in WReg to SPI   
   
      movf   ADDR2,w               ;moves second address bytes into WReg
      call   SPIMPolIsTransmitOver   ;waits until the bus the transmit is finished
      call   SPIMPolPut            ;feeds the wreg into SPI bus
      
      movf   ADDR1,w               ;moves the first bits into the Wreg
      call   SPIMPolIsTransmitOver   ;waits until the bus is cleared
      call   SPIMPolPut            ;feeds the wreg into SPI bus

;*** Latch in the new LED address data ***;
      bcf      PORTE,0               ;clears the output at PortE,0. output enable (on the A6832) goes down.
      call   SPIMPolIsTransmitOver   ;all data must be sent before the next instruction.
      bcf      PORTD,0               ;clears the SPI adress line, data is written into the A6832 latch
      bsf      PORTE,0               ;sets A6832 output active

;***address data is latched and enabled. It take 1µs for the A6832 to respond, or 10 instructions. ***;
      bcf      PORTE,2         ;clears the flip-flop register at display circuits
      bcf      PORTE,1         ;pulls down the clock pin on the flip-flop.
      bsf      PORTE,2         ;flip-flop cleared, return pin high

;*** test if the data to be written is for LEDs or the ASCII display logic ***;   
      movlw   0xfc         ;sets a timer preset into working register
      btfsc   DATA_WIDTH,2   ;I've decided to indicate ASCII info with the third bit in data width variable.
      movlw   0xff         ;new preset for the timer put into the work register. ASCII doesn't need the full time LEDs do.
      movwf   TMR0L         ;sets the new preset data from WReg into the lower TMR0 register.
      bcf      DATA_WIDTH,2   ;clears the bit so the data width is correct
      lfsr   1,LEDDATA      ;leddata is an assigned RAM register where the first byte of LED data is placed
      movf   INDF1,W         ;data in the LEDDATA is written to WReg

;*** A6832 latency should now be cleared. Time to latch in the address data ***;      
      bsf      PORTE,1               ;sets clock pulse high, flip-flop latches the data
      bsf      PORTD,0               ;pulls latch pin down on the 6832, readies it for new batch of data
      bcf      PORTE,1               ;clears clock pulse to flip-flop
      bcf      PORTE,0               ;Clears the blank bit, turns off the A6832 output again.
      
;*** send LED data into 6832 ***;
LED_LOOP:
      call    SPIMPolPut            ;sends data in WReg to SPI line
      dcfsnz    DATA_WIDTH,f         ;decreases value in data_width by one. Skip if result is not zero
      goto    FINISH               ;Zero is EOD, go to end.
      incf   FSR1L               ;increments the FSR1 to point the next data location
      movf   INDF1,w               ;loads the data in FSR1 into WReg
      call   SPIMPolIsTransmitOver   ;wait until the line is clear
      goto   LED_LOOP            ;loops back to the beginnin

;*** done, housekeeping left ***;
FINISH:
      bcf      PORTD,0            ;latch in the data
      bsf      PORTE,0            ;make the A6832 output active
      dcfsnz  DISPLAY_NUMBER,f   ;decreases the display number for the main routine to know who's next
      movff   MAX_DISPLAY_NUMBER,DISPLAY_NUMBER   ;If the number was zero, start over from highest number
      bsf      INTCON, INTF       ;clear interrupt flag
      movff   BSR_TEMP,BSR      ;restore BSR register
      movff   WREG_TEMP,WREG      ;restore working register
      movff   STATUS_TEMP,STATUS   ;restore STATUS register
      retfie

;******************************************************************************


If this seems too long for an interrupt, perhaps I should only manipulate the stack in the interrupt so it returns to this routine somewhere else in the code, and from there back to the original interrupt point, or is there a more direct way for that?


Wed Dec 09, 2009 10:21 am
Profile
Moderator
User avatar

Joined: Tue Jan 15, 2008 2:31 pm
Posts: 14659
Location: Home sweet home!
Firstly, EssEss, keep commenting if you have time and desire.

Secondly, KW1252, I've met EssEss, seen his code, discussed deepish tech with him in earnest, I know for a fact that he is bloody good at what he does and definitely to be respected. He has way Way WAY more embedded experience than I do, and that is an understatement, I don't even compare. He has some good reason to not be humble IMO. If you listen to what he has to say, it will likely take you far. Don't take offense to it, that's for sure, he's just trying to help, a real life proper nice guy! I promise. :-)

I didn't read the code, so can't comment either way, but I can say that some of my ISR code is rubbish and you do NEED to keep it short and hand off work to main loop code where possible.

Right, I'm out again, carry on :-)

Fred.

_________________
DIYEFI.org - where Open Source means Open Source, and Free means Freedom
FreeEMS.org - the open source engine management system
FreeEMS dev diary and its comments thread and my turbo truck!
n00bs, do NOT PM or email tech questions! Use the forum!
The ever growing list of FreeEMS success stories!


Wed Dec 09, 2009 10:36 am
Profile WWW
LQFP144 - On Top Of The Game
User avatar

Joined: Sat Feb 23, 2008 8:58 pm
Posts: 387
Waiting for a state on the SPI bus to change in an ISR is VERY bad. It doesn't matter if it's fast or not it doesn't make sense. You either do that in the main loop or have the SPI configured to generate an interrupt when it's ready and then you do the next transmission in that interrupt.

Jean


Wed Dec 09, 2009 4:53 pm
Profile WWW
LQFP112 - Up with the play
User avatar

Joined: Thu Sep 10, 2009 9:23 am
Posts: 244
Location: Dayton, OH
thanks fred, but he was right, and my skin is thicker than most :oops: ... but yes, I really was trying to help. thats why I like whiteboards with circles and arrows and high bandwidth conversations.

if you run this from mplab, I think it has a cycle accurate simulator which can tabulate your total time for you. I see lots of calls in there too, the stack on the 16's is h/w and the I think it's the same of 18's and it's a common stack (all from memory, I'll go visit the datsheets to make sure) .. so be careful how deep you go with these calls. lots of calls are also a leading indicator of bloat. I do as Jean has said, I try to have everything already setup and ready for the isr and the isr just does the simple lifting based off an event. All the heavy lifting is done from the mainline code unless I'm so resource constrained that I can't afford too - then I start re-evaluating my architecture. Somewhere along the way my 'ideal view' has diverged from what is really possible.

It's been about 5 years since I last visited a pic, so I'll go off and do some brushing up. Have you looked into the sourceboost compiler ? it's pretty slick and pennies on the dollar if you want to upgrade (I think it was $60usd for the full blown pkg). http://www.sourceboost.com/


Wed Dec 09, 2009 5:36 pm
Profile
LQFP112 - Up with the play
User avatar

Joined: Tue Jan 15, 2008 5:31 pm
Posts: 166
Must looks at that sourceboost thing more closely, looks pretty promising at the first glance, I'm already crapping my pants thinking about doing unit conversions with assembler.... not to mention the possible menu system using the ASCII display and a 5-way joystick.

The call routines are excessive, that's true. I just used the SPI library without thinking about too much. Using a subroutine call adds unnecessary latency, and all the library call really contains is a "movwf sspbuf" command plus a collision check... using "movff [data] sspbuf" directly in the routine would have less overhead and save a stack instruction. The stack is 31-deep in PIC18f though, so I'm not too worried about overflow there. Having the software to check for collisions would be "free" though, as there's 9 cycle delay after new data can be sent into SPI bus. That's not a whole lot to do anything else, though.

Quote:
I do as Jean has said, I try to have everything already setup and ready for the isr and the isr just does the simple lifting based off an event.


If I understood that right, that's the sort of thing I'm attempting here; the data is stored into the ADDRx and LEDDATA registers - all the procedure does is move them to the SPI as quickly as possible, with the relevant pin status changes.

I think I should explain how I planned the whole program to work:

The first stage and of least priority is the measuring part of the main loop reads the values from the ADC circuits, pulse counters or the SPI input, depending which quantity is being measured and then stored into memory. I think each quantity needs two banks of registers; one for working out an average between several measurings and the other the output value, you don't want to make a 7-segment display to update too rapidly. These measuring procedures are run continuously in order at the main loop.

The second stage with medium priority is a display interpreter procedure, which is run once between display writes. To detect if the write has taken place, the DATA_WIDTH value is read first. If this is zero, it means the display refresh routine has been run and needs a new set of information; otherwise exit. The segment counter calls a subroutine for each display element. The subroutine takes the information from the output data registers set by the corresponding measuring routine and interprets it into the proper form, which would be a bar segment, 7-segment, or ASCII character/location information. The subroutine then writes the data into ADDRx -registers which tell which LED element the data applies to, an the actual data into memory locations starting from LEDDATA. It then sets the DATA_WIDTH byte and returns.

Finally, there's the highest priority task, the display routine, triggered by the timer overflow. The routine sketch I've written already describes how I planned it to operate, but of course there's a few changes I've figured already; The routine first needs to check if the DATA_WIDTH byte is not zero; if it is, it means the refresh kicked in before or right in the middle of the interpreter cycle, and should exit. It should be really really unlikely situation though, bordering a malfunction. Also, the display routine should not bother with the display counter, it should be done in the interpreter routine.

What is really important here is the display routine is run as quickly as possible, at precisely the due time, at the top priority. The main concern is visibility and visual appearance. There's a lot of LED's to light up in sequence - 41 different sets in this outfit - so there's not much time for each to lit. The manufacturer approved overdrive is 160mA at 10% duty cycle at 1kHz refresh time; here's the current is 125mA, but the lit time per LED is 110µs per cycle. For even and flicker free lighting, it's important each get the same amount of lit time, at regular intervals. The way I see it can be done in the main loop is to modify the stack so that upon exiting the interrupt, the program accesses the display routine instead of interrupt point, runs the routine, then restores the registers and modifies the stack again so upon return from subroutine it resumes where interrupt was encountered. But then again, how will that differ from doing it from within the isr in the first place? I only have intuition and tutorials (and the datasheet manual of course) to go by on this, I certainly would appreciate good examples how these things are made.


Thu Dec 10, 2009 2:00 pm
Profile
Display posts from previous:  Sort by  
Reply to topic   [ 29 posts ]  Go to page Previous  1, 2, 3  Next

Who is online

Users browsing this forum: No registered users and 1 guest


You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum
You cannot post attachments in this forum

Search for:
Jump to:  
Powered by phpBB® Forum Software © phpBB Group
Designed by ST Software for PTF. ColorizeIt.