|
|
View previous topic :: View next topic |
Author |
Message |
JDBoyd
Joined: 11 Aug 2010 Posts: 4
|
Using largish structures |
Posted: Tue Aug 17, 2010 6:00 pm |
|
|
I'm having some trouble with using an array, which sounds really stupid. I ran into a similar problem earlier, but I just rearranged things until it started working correctly. Then 2 weeks later, I make a small seemingly unrelated change, and bam the problem is back, so I decided to cut the code out into an example program, that reproduces what I'm trying to do and the results I get.
I actually have two different examples here. Both examples use 128 bytes. One just uses a 128 byte array.
My data is 4 levels, 32 entries per level, and each entry takes 7 bits plus a dirty bit, so the other example uses structs and arrays. Both have trouble. If I read an element immediately after writing it, I get what I wrote, but if I read it later from a different function (say I read it correctly in the setter, but I read it incorrectly from 2 lines later in the function that called the setter) I get something different. There are no interrupts. The platform is a PIC18F46J50, so a few K of RAM. I'm using CCS PCH 4.110, 53371. I'm compiling from inside MPLAB version 8.53.
I include the two different methods since I've tried them both and they both misbehave. I'm hoping there is something obvious about PICs or CCS that I just don't know about. I also hope I'm not providing so much info that no one bothers reading the long post.
I've attached 4 files. Here are my structures and the code that reads/writes from them:
Code: |
#include "dls_hardware_config.h"
#include "dls_matrix.h"
int8 matrix2[128];
void dls_set2(int level, int input, int output)
{
matrix2[level * 32 + output] = input | 0x80;
}
int dls_get2(int level, int output)
{
return matrix2[level * 32 + output] & 0x7F;
}
#define MAX_CHNLS 32
//TODO: Stick data contents of this file into SRAM.
//Values are 0-127. The highbit is a dirty bit.
//128 slots for 32 outputs by 4 levels.
int8 input_cnt; //1-128
int8 output_cnt; //1-32
struct output
{
int is_dirty : 1;
int input : 7;
};
struct level
{
struct output outputs[MAX_CHNLS];
};
struct level matrix[4];
int dls_matrix_get_output_count(void)
{
return input_cnt;
}
int dls_matrix_get_input_count(void)
{
return output_cnt;
}
int8 dls_matrix_get_output(int8 level, int8 output)
{
int8 ret = matrix[level].outputs[output].input;
return ret;
}
void dls_matrix_init(int8 inputs, int8 outputs)
{
int8 i;
for(i=0; i<4; i++) // levels
{
int j;
for(j=0; j<32; j++)
{
matrix[i].outputs[j].input=0;
matrix[i].outputs[j].is_dirty=1;
}
}
input_cnt = inputs;
output_cnt = outputs;
}
void dls_matrix_set_output(int8 level, int8 output, int8 val)
{
matrix[level].outputs[output].input = val;
matrix[level].outputs[output].is_dirty = 1;
}
|
Here is the test code that will show the problem.
Code: |
#include "dls_hardware_config.h"
#include "dls_matrix.h"
void main( void )
{
setup_oscillator(OSC_PLL_ON);
setup_adc_ports(NO_ANALOGS|VSS_VDD);
setup_wdt(WDT_OFF);
setup_timer_0(RTCC_EXT_L_TO_H);
setup_timer_1(T1_FOSC | T1_DIV_BY_1);
enable_interrupts(GLOBAL);
setup_timer_2(T2_DISABLED,0,1);
setup_timer_3(T3_DISABLED|T3_DIV_BY_1);
setup_timer_4(T4_DISABLED,0,1);
setup_ccp1(CCP_OFF);
setup_comparator(NC_NC_NC_NC);// This device COMP currently not supported by the PICWizard
#define OUT_CNT 32
fprintf(remote_232, "\r\nStarting\r\n");
dls_matrix_init(32, OUT_CNT); //TODO: Read from output board.
fprintf(remote_232, "Past Init\r\n");
signed int l=3;
int i, input;
fprintf(remote_232, "Setting data\r\n");
for(l=0; l<4; l++)
for(i=0; i<OUT_CNT; i++)
{
dls_matrix_set_output(l, i, l);
dls_set2(l, i, l);
}
for(l=3; l>=0; l--)
for(i=0; i<OUT_CNT; i++)
{
input = dls_matrix_get_output(l, i);
fprintf(remote_232, "gu: l %d i %d o %d\r\n", l, input, i);
input = dls_get2(l, i);
fprintf(remote_232, "gu: l %d i %d o %d\r\n", l, input, i);
}
}
|
Now, here is some of the output from the test program. As you can see, the input should equal the level, and since this sample is for level 0, all inputs should be 0. You see the same sort of thing in the other levels where the input is not equal to level 1, 2, or 3.
Code: |
gu: l 0 i 16 o 17
gu: l 0 i 0 o 17
gu: l 0 i 16 o 18
gu: l 0 i 80 o 18
gu: l 0 i 16 o 19
gu: l 0 i 2 o 19
gu: l 0 i 16 o 20
gu: l 0 i 102 o 20
gu: l 0 i 16 o 21
gu: l 0 i 0 o 21
gu: l 0 i 16 o 22
gu: l 0 i 32 o 22
gu: l 0 i 16 o 23
gu: l 0 i 32 o 23
gu: l 0 i 16 o 24
gu: l 0 i 1 o 24
gu: l 0 i 16 o 25
gu: l 0 i 1 o 25
gu: l 0 i 16 o 26
gu: l 0 i 1 o 26
gu: l 0 i 16 o 27
gu: l 0 i 1 o 27
gu: l 0 i 16 o 28
gu: l 0 i 1 o 28
gu: l 0 i 16 o 29
gu: l 0 i 1 o 29
gu: l 0 i 16 o 30
gu: l 0 i 1 o 30
gu: l 0 i 16 o 31
|
For completeness, here is the header file that goes with the first file:
Code: |
#ifndef __DLS__MATRIX__H_
#define __DLS__MATRIX__H_
/**
*
*
* @param inputs is a value from 1 to 128
* @param outputs is a value from 1 to 32
*/
extern void dls_matrix_init(int inputs, int outputs);
extern void dls_matrix_set_output(int level, int idx, int val);
extern void dls_matrix_clear_output(int level, int idx);
extern int dls_matrix_get_output(int level, int idx);
extern int1 dls_matrix_is_dirty(int level, int idx); //Short is a 1 bit value
extern int dls_matrix_get_output_count(void);
extern int dls_matrix_get_input_count(void);
extern void dls_set2(int level, int input, int output);
extern int dls_get2(int level, int output);
#endif // __DLS__MATRIX__H_
|
And here is my header file for hardware configuration. I left in the bits for hardware that my project uses but the test program doesn't use.
Code: |
#ifndef __DLS_HARDWARE_CONFIG__H__
#define __DLS_HARDWARE_CONFIG__H__
#include <18F46J50.h>
#use delay(clock=48Mhz)
#FUSES NOWDT //No Watch Dog Timer
#FUSES WDT128 //Watch Dog Timer uses 1:128 Postscale
#FUSES HSPLL //High Speed Crystal/Resonator with PLL enabled not
#FUSES PLL3
#FUSES NOCPUDIV
// #FUSES DEBUG //Debug mode for use with ICD
// #FUSES NOXINST //Extended set extension and Indexed Addressing mode disabled (Legacy mode)
#FUSES STVREN //Stack full/underflow will cause reset
#FUSES NOPROTECT //Code not protected from reading
#FUSES FCMEN //Fail-safe clock monitor enabled
#FUSES IESO //Internal External Switch Over mode enabled
#FUSES IOL1WAY //Allows only one reconfiguration of peripheral pins
#FUSES NOWPCFG
#FUSES WPEND
#FUSES WPDIS
#FUSES NOLPT1OSC //Timer1 configured for higher power operation
#FUSES T1DIG
#FUSES MSSPMSK7
#FUSES DSWDT2147483648
#FUSES DSWDT
#FUSES DSBOR
#FUSES RTCOSC_T1
#FUSES DSWDTOSC_INT
#FUSES WPFP
#PRIORITY RDA2,TBE2,TBE,RDA,TIMER1
#use rs232(baud=9600,parity=N,xmit=PIN_C6,rcv=PIN_C7,bits=8, stream=remote_232)
#pin_select U2TX=PIN_C1
#pin_select U2RX=PIN_C0
#use rs232(baud=19200,parity=N,xmit=PIN_C1,rcv=PIN_C0,enable=PIN_C2,bits=8, stream=cp_485)
// Front Panel port
#use i2c(Master,fast=100000,sda=PIN_D1,scl=PIN_D0,FORCE_HW, STREAM= I2cFP)
// Matrix cards port
#use i2c(Master,fast=100000,sda=PIN_B5,scl=PIN_B4,FORCE_HW, STREAM= I2cMTX)
/* *********************************TIMER 1 Defines ****************** */
#define ONE_Ms 63536-45568 // 1,000,000 / 21Ns per clk @ 48Mhz
#endif // __DLS_HARDWARE_CONFIG__H__
| [/code] |
|
|
PCM programmer
Joined: 06 Sep 2003 Posts: 21708
|
|
Posted: Tue Aug 17, 2010 8:24 pm |
|
|
Can you post a much smaller test program ? For example, all of the
following code can be cut. It sets up peripheral modules. It doesn't
have anything to do with structure problems:
Quote: |
setup_oscillator(OSC_PLL_ON);
setup_adc_ports(NO_ANALOGS|VSS_VDD);
setup_wdt(WDT_OFF);
setup_timer_0(RTCC_EXT_L_TO_H);
setup_timer_1(T1_FOSC | T1_DIV_BY_1);
enable_interrupts(GLOBAL);
setup_timer_2(T2_DISABLED,0,1);
setup_timer_3(T3_DISABLED|T3_DIV_BY_1);
setup_timer_4(T4_DISABLED,0,1);
setup_ccp1(CCP_OFF);
setup_comparator(NC_NC_NC_NC);
|
Of the remaining code, if you can cut it down to 1/3 of its current
size, it would make it a lot easier for us to help you.
Quote: |
I include the two different methods since I've tried them both
|
Don't show us two examples. One will be enough. Post it as one
continuous program, and not in segments.
Here are examples of short programs concerning structures:
http://www.ccsinfo.com/forum/viewtopic.php?t=33458&start=1
http://www.ccsinfo.com/forum/viewtopic.php?t=31637&start=1
This one is a bit longer:
http://www.ccsinfo.com/forum/viewtopic.php?t=35500&start=1
Try to make it similar to those programs in terms of length. |
|
|
Wayne_
Joined: 10 Oct 2007 Posts: 681
|
|
Posted: Wed Aug 18, 2010 2:45 am |
|
|
One problem I see is that you are using the same name for both the output struct tag and a parameter in your function:-
Code: |
struct output
{
int is_dirty : 1;
int input : 7;
};
int8 dls_matrix_get_output(int8 level, int8 output)
|
Also level
Code: |
struct level
{
struct output outputs[MAX_CHNLS];
};
int8 dls_matrix_get_output(int8 level, int8 output)
|
Normally this may not be a problem but I don't know how the CCS compiler deals with it.
There are some other var names which clash but shouldn't cause a problem.
I also found it dificult to follow your output as you mix up your var names and output names. You also show data from somwhere near the end of the trace.
Reduce your array sizes so ALL the trace can fit on the page. Don't make the page too big either |
|
|
Ttelmah
Joined: 11 Mar 2010 Posts: 19559
|
|
Posted: Wed Aug 18, 2010 9:45 am |
|
|
Another problem is declaring variables inside the code.
In C, variables must be declared at the _start_ of a code section. You technically can declare them inside a subsection (bracketted), but it is not legitimate to declare them 'mid code'. The compiler ought to complain about this, but since it doesn't, I'd suspect these are incorrectly being allocated to the same memory areas as earlier declarations, and hence values are being overwritten....
Best Wishes |
|
|
FvM
Joined: 27 Aug 2008 Posts: 2337 Location: Germany
|
|
Posted: Wed Aug 18, 2010 9:48 am |
|
|
There are two issues with your code:
1. You confused the argument order with dls_set2(). The function definition expected in your code is:
Code: | void dls_set2(int level, int output, int input) |
2. CCS doesn't allow a combination of variable definition and value assignment other than constant initializers.
Code: | int8 ret = matrix[level].outputs[output].input; |
Write instead
Code: | int8 ret;
ret = matrix[level].outputs[output].input; |
|
|
|
JDBoyd
Joined: 11 Aug 2010 Posts: 4
|
|
Posted: Wed Aug 18, 2010 8:47 pm |
|
|
PCM, I will continue to try to create a short example. Generally, short examples don't want to exhibit the same behavior as the larger program.
To everyone else, I will go through and apply the suggestions about not using C99 style variable definitions in the middle of a block, and not mixing variable declaration with value assignment, as well as making sure that type names and variable names never overlap (although, it seems rather crummy if PCH can't handle that).
You are correct about the mixed up argument order, but that was not the case in the main project's code, only in the test program. |
|
|
FvM
Joined: 27 Aug 2008 Posts: 2337 Location: Germany
|
|
Posted: Wed Aug 18, 2010 10:47 pm |
|
|
Quote: | Generally, short examples don't want to exhibit the same behavior as the larger program. | In this rare cases you can hardly expect help from the forum.
Quote: | as well as making sure that type names and variable names never overlap (although, it seems rather crummy if PCH can't handle that). |
Don't worry about this point. The test code gives correct result after fixing the two reported issues.
Quote: | but that was not the case in the main project's code, only in the test program | It's the one and only reason, why the dls_set/get variant shows incorrect results. Do you say,
you didn't even check the results of your test code? |
|
|
|
|
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
|