Post by thread:[PIC]Puzzled by Microchip AN889 VF motor control

I have been asked by someone to help with a problem they're having with the uChip example code in this app note, which I think I've sorted. Anyhow, I'm really puzzled by their macros which seem back to front to me. Here is the macro for 8*8 unsigned multiply MULT MACRO BIT btfsc no_lsb,bit ; OK no problem here. addwf result_msb,f ; here neither. rrf result_msb,f ; this appears to be dividing to me. rrf result_lsb,f ENDM The divide routine is longer but uses rlf. Now as carry isn't used to bring the result of rrf MSB back to the top bit of the byte, I fail to see how this is a multiply. Going on further the coding of the interrupt routine seems somewhat farinacious - it has goto's that branch to goto's that call a routine, that branch back into the interrupt routine, very hard to follow. I assume they've done this for speed purposes. Example inside the interrupt. BANKSEL PIR1 btfsc INTCON,RBIF goto CHECK_FAULT ; this is outside the interrupt routine btfsc PIR1, TMR1IF goto TIMER1_OVERFLOW ; as is this <snip> <snip> RETFIE CHECK_FAULT <snip> goto NO_ACTION ; this is in this block of code call RUN_MOTOR_AGAIN goto POP_UP ; this takes us back into the interrupt routine Now, RUN_MOTOR_AGAIN itself has a couple of goto's and a call. Or am I just having an off day? Colin -- cdb, on 14/04/2009

cdb wrote: > Anyhow, I'm really puzzled by their macros which seem back to front to > me. > > Here is the macro for 8*8 unsigned multiply > [snip] (looks at the code from the AN889 zip file) That is seriously nasty code. Talk about breaking every rule in the book... Hard-coded config word values (i.e. _CONFIG 0x3FFF instead of _CONFIG WDT_OFF & ...), macros completely devoid of comments as to their function (or the function of their parameters)... And $DEITY help you if that code ever gets large enough to occupy more than one program memory page. I wonder if this guy has been reading "How to Write Unmaintainable Code" and treated it as a how-things-should-be-done guide? I've seen better code in "Programming For Dummies" type books. > Now as carry isn't used to bring the result of rrf MSB back to the top > bit of the byte, I fail to see how this is a multiply. It looks like it's part of an 8x8 multiply (looks more like a 1x8 or something like that), but the 'rrf's make it run the wrong way. > Going on further the coding of the interrupt routine seems somewhat > farinacious - it has goto's that branch to goto's that call a > routine, that branch back into the interrupt routine, very hard to > follow. I've seen tidier plates of spaghetti. > I assume they've done this for speed purposes. Doubt it. Code reuse is all well and good, but the interrupt routine should ideally not be CALLing other chunks of code. Also I'd either put it in a separate module (if using object/linker mode) or ORG it at 0x04 and let the MAIN routine have everything after the ISR RETURNs. Calling the "return from ISR" block POPUP is madness. > Example inside the interrupt. [snip nastiness] I'd have probably written it a bit like this: ISR_ENTRY: MOVWF W_SAVE ; save state SWAPF STATUS, W MOVWF STATUS_SAVE BTFSC PIR1, RBIF ; interrupt from SOME unit GOTO RBIF_INTERRUPT GOTO ISR_EXIT ; no interrupts (?), exit ISR RBIF_INTERRUPT: ; handle the PORTB interrupt ; ... GOTO ISR_EXIT ; exit ISR ISR_EXIT: SWAPF STATUS_SAVE, W ; restore state MOVWF STATUS SWAPF W_SAVE, F SWAPF W_SAVE, W RETFIE ; exit from interrupt > Or am I just having an off day? Nope. It's not the worst code I've seen (hint: never offer to help another development team try and find bugs in code that's been written by Broadcom) but it isn't far off. I do, however, find it mildly amusing that Microchip don't even follow their own code-style recommendations. What's even more amusing is that this is appnote code. It does make a pretty good example of "how not to do it", though. -- Phil. spam_OUTpiclistTakeThisOuTphilpem.me.uk http://www.philpem.me.uk/

> Going on further the coding of the interrupt routine seems > somewhat farinacious - it has goto's that branch to goto's that > call a routine, .... >From my brief stint as a stand-in food tech I know that farinaceous means "of the nature of a cereal or grain products" Are you confusing the style with "floury" rather than "flowery" ? ;-)) BTW, you can see holes in some of Microchip's code if you hold it up to the light. I've tried a few of their examples and templates and thought they were just naff. Some do have big hairy errors in them. Although the last one I used, TB028 (convert date to day name) works well enough If you understand what you want to accomplish it's often quicker to start from scratch and write it in your own style

:: Are you confusing the style with "floury" rather than "flowery" ? Nasturtiams to you too! :) :: you can see holes in some of Microchip's code if you hold :: it up to the light Which is a problem as many beginners would assume that code from the 'source' would be correct and typical. Perhaps they'll get the Hitech chaps and chapesses to check application code notes from now on. I'm led to believe that this code is actually the same as shipped with the motor control development board. Colin -- cdb, .....colinKILLspam@spam@btech-online.co.uk on 15/04/2009 Web presence: http://www.btech-online.co.uk Hosted by: http://www.1and1.co.uk/?k_id=7988359

:: I do, however, find it mildly amusing that Microchip don't even :: follow their own code-style recommendations. I would have expected them to have some sort of 'house style' templates, and to have tested the code in hardware. Even worse the code doesn't quite follow what is in the text, it doesn't for example make use of the accelerate/decelerate declarations, (which is what was causing the person who contacted me problems), the sine table values don't match the angles stated (apart from 270 deg) and one howler is where it tells one to comment out an #ifdef statement, that actually requires to be 'commented in' ! Colin -- cdb, colinKILLspambtech-online.co.uk on 15/04/2009 Web presence: http://www.btech-online.co.uk Hosted by: http://www.1and1.co.uk/?k_id=7988359

Many of their app notes used to be written by guests (I assume these were actually contracted to do so, but their company name was mentioned in the app notes). I wonder if that could be the case with this one? Sean On Tue, Apr 14, 2009 at 3:50 PM, cdb <.....colinKILLspam.....btech-online.co.uk> wrote: {Quote hidden}> > > :: I do, however, find it mildly amusing that Microchip don't even > :: follow their own code-style recommendations. > > I would have expected them to have some sort of 'house style' > templates, and to have tested the code in hardware. > > Even worse the code doesn't quite follow what is in the text, it > doesn't for example make use of the accelerate/decelerate > declarations, (which is what was causing the person who contacted me > problems), the sine table values don't match the angles stated (apart > from 270 deg) and one howler is where it tells one to comment out an > #ifdef statement, that actually requires to be 'commented in' ! > > Colin > -- > cdb, EraseMEcolinspam_OUTTakeThisOuTbtech-online.co.uk on 15/04/2009 > > Web presence: http://www.btech-online.co.uk > > Hosted by: http://www.1and1.co.uk/?k_id=7988359 > > > > > > > >

On Apr 14, 2009, at 4:26 PM, cdb wrote: > It has after the authors name Microchip technology Inc, and I think > I've seen the same name on other code examples. I always figured that Microchip is a CHIP (and hardware) company, and the App notes were not too atypical of what happens when you let hardware engineers and chip designers write software. I've seen similar, sorta all too often... You have to figure that compared to a company like Microsoft or cisco, Microchip (with a grand total of under 5000 employees), has very few people that specialized in writing software. And most of them are working on MPLAB, or compilers, or other "critical" tools, and don't have the hardware background to write the interesting app notes anyway. I've occasionally thought that it would be a great job, working for a microcontroller vendor and just churning out app note after app note, diving into assorted new technologies just deeply enough to get educational demonstration code working and documented, showing off the companies technology and helping their customers without a lot of monotonous upkeep of an actual software product. Unlike what seems to happen today, you could publish new versions for new chips ("remember AN1234? Well here's what it would look like on a PIC24!") These days you could collaborate with assorted people on the net "Hey Olin - great macro library; can we publish it as an app note? Credit would stay with you of course, and we'll send some product your way as a reward - a sort of mini-contest instead of these big ones we have occasionally." (not everyone would say yes, of course, but it's not an obviously silly idea, IMO.) It'd be a near-ideal telecommuter job, too. (I think in the real world, people who can do this sort of thing get sucked into helping actual specific customers who have been checkbooks. "Application Engineers." But it's not clear to me that that couldn't be a separate job...) BillW

:: I've occasionally thought that it would be a great job, working :: for a :: microcontroller vendor and just churning out app note after app :: note, :: diving into assorted new technologies just deeply enough to get :: educational demonstration code working and documented I'd definitely go for that job! Actually I thought they did indeed employ people in their customer service/engineers department to do just this. I was under the impression that people like Lucio De Jasio and Creed Hudlestone (sp?) were Microchip employees or contracted to Microchip and their books were just spin offs. I know Parallax do, though of course they aren't a manufacturer, so would that be the same for Texas, Dallas and others? Colin -- cdb, @spam@colinKILLspambtech-online.co.uk on 15/04/2009 Web presence: http://www.btech-online.co.uk Hosted by: http://www.1and1.co.uk/?k_id=7988359