 |
 |
| View previous topic :: View next topic |
| Author |
Message |
mbge5amw
Joined: 13 Dec 2004
Posts: 15
Location: Yorkshire, UK
|
| variables being overwritten - 2.236 - PIC18F258 - can bus |
 Posted: Tue Feb 20, 2007 7:56 am |
 |
|
I have tried to recreate this in a smaller project but have been unable, I have therefore picked out what I think may be the relevant parts of this project to see if anyone can see what is going wrong.
99% of the time it is running fine, however I appear to be getting memory corruption.
I have found talk on the message board of problems with certain compilers overwriting data with the can bus routines but I am using 2.236 which sounds to be the most stable.
I have a motor control project which basically goes like this.
Incoming data on CAN bus is buffered in interrupt routine.
Main loop then decodes this data to work out required motor speed, using motor_convert_100_12 amongst other routines.
Timer generates interrupts at varying rate to cause next step to be written to stepper motor.
Also when a sensor is triggered the current position is stored in memory ready for the main loop to check that the motor is where it is expected to be.
The memory that gets corrupted is the first 2 elements of the array motor_fast_step_size_lookup[] i.e. addresses 02A and 02B
The value of 02B after corruption just happens to be the variable passed to motor_convert_100_12() and this routine uses addresses 22A and 22B for data manipulation.
I have confirmed that I am not writing values to the array motor_fast_step_size_lookup[] through errant code - this is normally written once on power up and then referred to from then on.
I imagine that for some reason the bank address for data writing is getting corrupted and changed from 2 to 0, but I cannot see why.
Am I right in thinking that the compiler should handle all this, I am a bit stuck now to see what I can do.
If you wish to see any other aspects of the files I can provide the relevant info.
Thanks in advance for your help.
Andy
| Code: |
begin P3M00.C
#include "18F258.h"
#device *=16
#DEVICE ADC=8 // 8 bit resolution is enough for our purposes, and maintains compatability with other devices as well as using less memory
#define CLOCK_USED_MHZ 20
#define CLOCK_USED 20000000
#use delay(clock=CLOCK_USED, restart_wdt)
#use rs232 (baud=57600, xmit=PIN_C6, rcv=PIN_C7, ERRORS)
#use i2c (master, sda=PIN_C4, scl=PIN_C3, FORCE_HW)
#define _I2C_W_ADDRESS_ 0x5A
#fuses HS, WDT,NODEBUG,NOLVP,PUT
/**********************************************************/
/* Name: motor_datum_isr
/* Purpose: store details of speed travelling at when sensor passed and abs pos read at the time
/* this will be decoded when the processor has time and abs pos modified if needbe
/* Design status: incomplete
// amw - just need to verify if force_stop is needed on preset control, may stop us reaching presets above vertical
/**********************************************************/
#int_ext
void motor_datum_isr (void)
{
if (motor_datum_retrigger_step_size == 0) // if already have data to decode then decode that,
{ // don't read any new data
motor_datum_retrigger_pos = motor_act_step_pos;
motor_datum_retrigger_microstep = motor_micro_step_pos;
motor_datum_retrigger_timer_reload = motor_timer_actual_period;
motor_datum_retrigger_step_size = motor_actual_step_size;
motor_datum_retrigger_direction = motor_actual_direction;
motor_datum_retrigger_prescaler = motor_actual_prescale;
motor_datum_retrigger_postscaler = motor_actual_postscale;
}
if (frame_axis == TILT)
{
if (motor_finding_upper_limit == FALSE) // if we are not deliberately trying to reach limit
if (motor_actual_direction < 0) // and moving upwards
motor_force_stop = TRUE;
}
}
/**********************************************************/
/* Name: motor_timer_isr
/* Purpose: call external routines to reload timer and write step
/* Design status: appears complete - untested
/**********************************************************/
#ifndef _NO_REAL_STEPS_ //
#int_timer1 //
#endif
void motor_timer_isr (void) //
{ //
static signed int8 postscale_counter = 0; //
int16 local_timer_reload; //
local_timer_reload = (0xFFFF - motor_timer_actual_period);
set_timer1 (local_timer_reload); // set timer ready for next step
<snip> (lengthy series of branches and calcultaions to move motor by required amount and calculate required delay before calling timer again)
if (get_timer1 () < (local_timer_reload)) // if ISR latency was so long it missed a trigger
{ //
set_timer1 (0xFC00); // so try again ASAP (with a little time left to process program loop stuff and decode comms)
} //
} //
} //
/**********************************************************/
/* Name: motor_convert_100_12
/* Purpose: take a number in the range -100 to +100 and convert it into range -12 to +12
/* Design status: corrupts data at 02A and 02B courteousy of scratch data at 22A perhaps?
/**********************************************************/
signed int8 motor_convert_100_12 (signed int8 input_speed)
{
signed int8 local_speed;
local_speed = input_speed;
if (local_speed > 100)
local_speed = 100;
if (local_speed < -100)
local_speed = -100;
return (int8) ( local_speed * 12L / 100); // convert from full scale into -12 > +12
}
end P3M00.C
|
| Code: |
begin OUTPUT
Executing: "C:\Program Files\PICC\PICC PCH 3_236\Ccsc.exe" "P3M00.C" +FH +DF +LN +T -A +M +Z +Y=9 +EA
>> Warning 216 "E:\work\p3m00\P3M00.C" Line 150(0,1): Interrupts disabled during call to prevent re-entrancy: (@DIV88)
>> Warning 216 "E:\work\p3m00\P3M00.C" Line 150(0,1): Interrupts disabled during call to prevent re-entrancy: (@MUL1616)
>> Warning 202 "E:\work\p3m00\device.h" Line 16(7,10): Variable never used: rs232_errors
Memory usage: ROM=93% RAM=33% - 50%
0 Errors, 3 Warnings.
end OUTPUT
|
| Code: |
begin P3M00.SYM
000 @SCRATCH
001 @SCRATCH
001 _RETURN_
002 @SCRATCH
003 @SCRATCH
004 @SCRATCH
005 @INTERRUPT_AREA
006 @INTERRUPT_AREA
007 @INTERRUPT_AREA
008 @INTERRUPT_AREA
009 @INTERRUPT_AREA
00A @INTERRUPT_AREA
00B @INTERRUPT_AREA
00C @INTERRUPT_AREA
00D @INTERRUPT_AREA
00E @INTERRUPT_AREA
00F @INTERRUPT_AREA
010 @INTERRUPT_AREA
011 @INTERRUPT_AREA
012 @INTERRUPT_AREA
013 @INTERRUPT_AREA
014 @INTERRUPT_AREA
015 @INTERRUPT_AREA
016 @INTERRUPT_AREA
017 @INTERRUPT_AREA
018 @INTERRUPT_AREA
019 rs232_errors
01A motor_force_stop
01B motor_report_locate_complete
01C motor_repeat_locate
01D motor_select_next_function_flag
01E motor_update_current_flag
01F motor_unattainable_step_flag
020 motor_finding_upper_limit
021 motor_force_pfd
022 motor_modify_current_flag
023 motor_current_level
024-029 motor_slow_step_size_lookup
02A-02F motor_fast_step_size_lookup
<snip>
229 motor_config.@SCRATCH
22A motor_convert_100_12.local_speed
22A motor_config_8.memory_location
22A motor_config_16.memory_location
22A motor_calibrate_if_required.@SCRATCH
22A motor_report_boot_up.@SCRATCH
22A motor_config.@SCRATCH
22B-22C motor_config_16.required_value
22B-22C motor_process_datum.expected_pos
22B-22C motor_config_8.required_value
22B motor_convert_100_12.@SCRATCH
22B motor_config.@SCRATCH
22C motor_convert_100_12.@SCRATCH
22C motor_config.@SCRATCH
<snip>
Compiler Settings:
Processor: PIC18F258
Pointer Size: 16
ADC Range: 0-255
Opt Level: 9
Short,Int,Long: 1,8,16
<snip>
end P3M00.SYM
|
| Code: |
begin P3M00.LST
CCS PCH C Compiler, Version 3.236, 31524 20-Feb-07 13:04
Filename: P3M00.LST
ROM used: 30576 bytes (93%)
Largest free fragment is 2192
RAM used: 512 (33%) at main() level
773 (50%) worst case
Stack: 21 worst case (17 in main + 4 for interrupts)
<snip>
.................... /**********************************************************/
.................... /* Name: motor_datum_isr
.................... /* Purpose: store details of speed travelling at when sensor passed and abs pos read at the time
.................... /* this will be decoded when the processor has time and abs pos modified if needbe
.................... /* Design status: incomplete
.................... // amw - just need to verify if force_stop is needed on preset control, may stop us reaching presets above vertical
.................... /**********************************************************/
.................... #int_ext
.................... void motor_datum_isr (void)
.................... {
.................... if (motor_datum_retrigger_step_size == 0) // if already have data to decode then decode that,
*
0BDC: MOVLB 1
0BDE: MOVF x23,F
0BE0: BNZ 0C06
.................... { // don't read any new data
.................... motor_datum_retrigger_pos = motor_act_step_pos;
0BE2: MOVFF 155,120
0BE6: MOVFF 154,11F
.................... motor_datum_retrigger_microstep = motor_micro_step_pos;
0BEA: MOVFF 156,127
.................... motor_datum_retrigger_timer_reload = motor_timer_actual_period;
0BEE: MOVFF 131,122
0BF2: MOVFF 130,121
.................... motor_datum_retrigger_step_size = motor_actual_step_size;
0BF6: MOVFF 143,123
.................... motor_datum_retrigger_direction = motor_actual_direction;
0BFA: MOVFF 149,124
.................... motor_datum_retrigger_prescaler = motor_actual_prescale;
0BFE: MOVFF 145,125
.................... motor_datum_retrigger_postscaler = motor_actual_postscale;
0C02: MOVFF 147,126
.................... }
....................
.................... if (frame_axis == TILT)
0C06: MOVF x5D,W
0C08: SUBLW 02
0C0A: BNZ 0C1A
.................... {
.................... if (motor_finding_upper_limit == FALSE) // if we are not deliberately trying to reach limit
0C0C: MOVF 20,F
0C0E: BNZ 0C1A
.................... if (motor_actual_direction < 0) // and moving upwards
0C10: BTFSC x49.7
0C12: BRA 0C16
0C14: BRA 0C1A
.................... motor_force_stop = TRUE;
0C16: MOVLW 01
0C18: MOVWF 1A
.................... }
.................... }
<snip>
.................... /**********************************************************/
.................... /* Name: motor_timer_isr
.................... /* Purpose: call external routines to reload timer and write step
.................... /* Design status: appears complete - untested
.................... /**********************************************************/
.................... #ifndef _NO_REAL_STEPS_ //
0C1A: BCF FF2.1
0C1C: MOVLB 0
0C1E: GOTO 008A
.................... #int_timer1 //
.................... #endif
.................... void motor_timer_isr (void) //
.................... { //
.................... static signed int8 postscale_counter = 0; //
.................... int16 local_timer_reload; //
....................
.................... local_timer_reload = (0xFFFF - motor_timer_actual_period);
*
1CEE: MOVLW FF
1CF0: BSF FD8.0
1CF2: MOVLB 1
1CF4: SUBFWB x30,W
1CF6: MOVLB 2
1CF8: MOVWF xE9
1CFA: MOVLW FF
1CFC: MOVLB 1
1CFE: SUBFWB x31,W
1D00: MOVLB 2
1D02: MOVWF xEA
....................
.................... set_timer1 (local_timer_reload); // set timer ready for next step
1D04: MOVFF 2EA,FCF
1D08: MOVFF 2E9,FCE
....................
<snip> (lengthy series of branches and calcultaions to move motor by required amount and calculate required delay before calling timer again)
.................... if (get_timer1 () < (local_timer_reload)) // if ISR latency was so long it missed a trigger
2006: MOVF FCE,W
2008: MOVFF FCF,03
200C: MOVLB 2
200E: MOVWF xEB
2010: MOVFF 03,2EC
2014: MOVF xEC,W
2016: SUBWF xEA,W
2018: BNC 2028
201A: BNZ 2022
201C: MOVF xE9,W
201E: SUBWF xEB,W
2020: BC 2028
.................... { //
.................... set_timer1 (0xFC00); // try again ASAP (with a little time left to process program loop stuff and decode comms)
2022: MOVLW FC
2024: MOVWF FCF
2026: CLRF FCE
2028: MOVLB 1
.................... } //
.................... } //
.................... } //
<snip>
.................... /**********************************************************/
.................... /* Name: motor_convert_100_12
.................... /* Purpose: take a number in the range -100 to +100 and convert it into range -12 to +12
.................... /* Design status: corrupts data at 02A and 02B courteousy of scratch data at 22A perhaps?
.................... /**********************************************************/
.................... signed int8 motor_convert_100_12 (signed int8 input_speed)
.................... {
.................... signed int8 local_speed;
....................
.................... local_speed = input_speed;
*
54DE: MOVFF 229,22A
....................
.................... if (local_speed > 100)
54E2: MOVLB 2
54E4: BTFSC x2A.7
54E6: BRA 54F2
54E8: MOVF x2A,W
54EA: SUBLW 64
54EC: BC 54F2
.................... local_speed = 100;
54EE: MOVLW 64
54F0: MOVWF x2A
.................... if (local_speed < -100)
54F2: MOVF x2A,W
54F4: XORLW 80
54F6: SUBLW 1B
54F8: BNC 54FE
.................... local_speed = -100;
54FA: MOVLW 9C
54FC: MOVWF x2A
....................
.................... return (int8) ( local_speed * 12L / 100); // convert from full scale into -12 > +12
54FE: CLRF 03
5500: MOVF x2A,W
5502: MOVWF 00
5504: BTFSC FE8.7
5506: DECF 03,F
5508: MOVWF x2B
550A: MOVFF 03,22C
550E: CLRF 18
5510: BTFSC FF2.7
5512: BSF 18.7
5514: BCF FF2.7
5516: MOVFF 03,2F2
551A: MOVWF xF1
551C: CLRF xF4
551E: MOVLW 0C
5520: MOVWF xF3
5522: MOVLB 0
5524: CALL 0E3C
5528: BTFSC 18.7
552A: BSF FF2.7
552C: MOVFF 02,22D
5530: MOVFF 01,22C
5534: MOVFF 02,22F
5538: MOVFF 01,22E
553C: MOVLB 2
553E: CLRF x31
5540: MOVLW 64
5542: MOVWF x30
5544: MOVLB 0
5546: BRA 5462
5548: MOVF 01,W
.................... }
554A: RETLW 00
....................
<snip>
Configuration Fuses:
Word 1: 2200 HS NOOSCSEN
Word 2: 0F0E BROWNOUT WDT128 WDT BORV20 PUT
Word 3: 0000
Word 4: 0081 STVREN NODEBUG NOLVP
Word 5: C00F NOPROTECT NOCPD NOCPB
Word 6: E00F NOWRT NOWRTD NOWRTB NOWRTC
Word 7: 400F NOEBTR NOEBTRB
end P3M00.LST
|
|
|
 |
PCM programmer
Joined: 06 Sep 2003
Posts: 21708
|
|
| Posted: Tue Feb 20, 2007 11:02 pm |
|
|
You have an array that's getting stepped on. Your believe that it's
caused by some sort of addressing problem.
Here's something to try. Rename the original array by appending
"_old" onto the end of it. Leave it in its present location as a place
holder. Then create a new version of the array (with the original
name) up in an out-of-the-way RAM location, say, up at 0x500.
Use the #locate directive to do this. Example:
| Quote: |
// Rename the original array to "old".
int8 motor_fast_step_size_lookup_old[6];
.
.
.
// Add these two lines at the end of all your existing declarations.
int8 motor_fast_step_size_lookup[6];
#locate motor_fast_step_size_lookup = 0x500 |
Then see if you still have the same problem. |
|
|
mbge5amw
Joined: 13 Dec 2004
Posts: 15
Location: Yorkshire, UK
|
|
| Posted: Wed Feb 21, 2007 2:37 am |
|
|
I have made that modification exactly as described above and the fault remains affecting the same location,
ie motor_fast_step_size_lookup_old[] (at 0x002A)is getting overwritten
but motor_fast_step_size_lookup[] (at 0x0500) remains intact so far as I can tell.
This seems to be a good fix for now, as my genuine data is uncorrupted, but I remain worried that the cause of this data corruption may also affect another location that I can't spot so easily and thus lead to niggling bugs remaining in the code.
As a crude workaround, is there perhaps a switch that will force the compiler to always specify the memory bank before accessing memory, or to always use 16 bit addressing. I realise this would probably slow the code down but this may be a necessary sacrifice in order to ensure reliability.
TIA
Andy |
|
|
PCM programmer
Joined: 06 Sep 2003
Posts: 21708
|
|
| Posted: Wed Feb 21, 2007 3:16 am |
|
|
I agree that the cause should be found. The real way to find it would be
to use an ICE and trigger it when a memory write occurs to 0x2A.
Without an ICE it probably has to be done by inspecting the .LST file,
so a smaller program would make it easier. |
|
|
mbge5amw
Joined: 13 Dec 2004
Posts: 15
Location: Yorkshire, UK
|
|
| Posted: Wed Feb 21, 2007 3:50 am |
|
|
I could take a look at shrinking the code but this precise problem has only come to light in the latest stages of code development, I am concerned that shrinking the code would simply make it go away as I would take away the exact set of triggers.
A few ideas I have:
1. Is the memory bank setting, as set by MOVLB, automatically stored in the stack during interrupts? Perhaps a stack overflow due to numerous interrupts and deep subroutines? I'll have a search on the forum to find some more info on this and see what looks relevant to my project.
EDIT - not this as I already had the STVREN fuse to protect from this
2. ICE: I have an ICD-U40 but using the command line PCM compiler. Do I need to upgrade to PCWH or do I also need to upgrade the hardware too - as far as I recall the ICD allows you to put break points in code but not conditional breaks.
3. Try very slimline code but keeping all the existing interrupt routines.
4. Check for any hardware errors as described in http://www.ccsinfo.com/forum/viewtopic.php?t=27543&highlight=silicon+bug
| Quote: | | Silicon bug in the chip - some 18F series had a problem where they skipped an address when returning from an interrupt. This situation happened in applications that used both high and normal priority interrupts together - check the errata for the chip you are using |
I'll begin afresh with my bughunting and report back on what I found
Cheers for your help thus far.
Andy |
|
|
mbge5amw
Joined: 13 Dec 2004
Posts: 15
Location: Yorkshire, UK
|
|
| Posted: Thu Feb 22, 2007 2:56 am |
|
|
OK, I'm not sure now if the latest change I made (while systematically reducing the code size) has rectified the fault or simply masked the symptoms but it certainly looks to be behaving itself at the moment.
FYI what I found was a problem with my CAN bus comms.
I have written a routine that takes the latest CAN bus command and puts it into a FIFO buffer.
This routine first checks to see if there is space in the FIFO buffer, if there isn't space then it is left in the hardware buffer until space is made available - the idea being that the hardware will inform the transmitting device that it has no more room and so it should try later.
This routine is called from each of the two CAN bus interrupts in order to get the fastest response possible most of the time.
It is also called from the main loop in order to process any commands that were left in the hardware buffer during the ISR due to a full FIFO buffer.
I did not want to have any of the variables used in this routine stomping on each other due to reentrancy so defined the code as inline so that 3 separate instances were made of it.
Unfortunately I overlooked the fact that the final destination - i.e. the FIFO buffer was a global variable and this and its write pointer was referred to by all three instances of the routine.
I have now removed the inline definition and the compiler is helpfully disabling interrupts while this is called in order to prevent reentrancy (with the added bonus of saving me a significant amount of code space)
Ideally I would like to handle this myself with a flag that prevents recurrent execution of the code within the routine, thus I can still have the timer interrupts occuring in order to write steps to the stepper motor but that is a separate issue which I ought to be able to work out for myself.
Thanks for listening to my ramblings.
Andy |
|
|
|
|
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 vote in polls in this forum
|
Powered by phpBB © 2001, 2005 phpBB Group
|
FAQ 
Search
Register
Profile 
Log in to check your private messages 
Log in
