 |
 |
| View previous topic :: View next topic |
| Author |
Message |
rikotech8
Joined: 10 Dec 2011
Posts: 376
Location: Sofiq,Bulgariq
|
| Cant configure function to work |
 Posted: Tue Jan 20, 2015 7:02 am |
 |
|
Hi everyone.
I am trying to interface MCP4822 digital to analog converter. Therefore I want to create a function that specifies the DAC(DAC_A or DAC_B), GAIN, and enabling/disabling selected DAC.
So the function definition looks like this:
| Code: | void spi_data_write(unsigned int16 data, int1 dac, int1 gain, int1 output_enable)
{
unsigned int16 spi_word = 0;
spi_word = ( data | (dac << 15) | (gain << 13) | (output_enable << 12) );
spi_xfer(spi_word);
}
|
The problem is that I am not able to set any bit except those presenting data.
Here is the entire code:
| Code: | #include <16f684.h>
#fuses INTRC_IO, NOWDT, NOMCLR
#USE delay(clock = 4M)
#use spi(MODE=0, DO=PIN_C2, CLK=PIN_C1, ENABLE=PIN_C0, BITS=16)
#define DAC_A 0
#define DAC_B 1
#define GAIN_1 1
#define GAIN_2 0
#define OUT_ENABLE 1
#define OUT_DISABLE 0
void chip_init();
void spi_data_write(unsigned int16 data, int1 dac, int1 gain, int1 output_enable);
void main()
{
chip_init();
while(TRUE)
{
spi_data_write(0x1, DAC_B, GAIN_1, OUT_ENABLE);
delay_us(1000);
}
}
void chip_init()
{
setup_oscillator(OSC_4MHZ);
}
void spi_data_write(unsigned int16 data, int1 dac, int1 gain, int1 output_enable)
{
unsigned int16 spi_word = 0;
spi_word = ( data | (dac << 15) | (gain << 13) | (output_enable << 12) );
spi_xfer(spi_word);
} |
The output of this code is:
https://drive.google.com/file/d/0B5Y7MPFIDn9dU0pLeDZaVG54dE0/view?usp=sharing
When I replace the arguments of the function inside the body of the function with corresponding numbers it works.
This way I dont have such issue:
| Code: | | spi_word = ( data | (1<< 15) | (1<< 13) | (1<< 12) ); |
With the above line the program works as expected:
https://drive.google.com/file/d/0B5Y7MPFIDn9dOWlLY1poV2VIQWs/view?usp=sharing
It looks like the function desnt accept the arguments
I don't have idea what's wrong. Please help 
_________________
A person who never made a mistake never tried anything new. |
|
 |
PCM programmer
Joined: 06 Sep 2003
Posts: 21708
|
|
| Posted: Tue Jan 20, 2015 12:30 pm |
|
|
Well, how to solve this problem ? You suspect there is something
wrong with the line that does a left-shift with variables. Therefore,
put in test code to work on that problem.
Start by commenting out the spi transfer code in your routine below.
We don't care about the transfer. We want to study the shifting.
Next, put in some test values for the function parameters. Put them
right inside the routine, as shown. Then put in a printf statement.
| Code: | void spi_data_write(unsigned int16 data, int1 dac, int1 gain, int1 output_enable)
{
unsigned int16 spi_word = 0;
dac = 1;
data = 0;
spi_word = (data | (dac << 15));
printf("%lx \r", spi_word);
// spi_word = ( data | (dac << 15) | (gain << 13) | (output_enable << 12) );
// spi_xfer(spi_word);
} |
Then modify your main() so it only calls the routine one time:
| Code: | void main()
{
chip_init();
// while(TRUE)
// {
spi_data_write(0x1, DAC_B, GAIN_1, OUT_ENABLE);
// delay_us(1000);
// } |
Next, change the PIC to a similar one that has a hardware UART. Also
add the #use rs232() statement:
| Code: |
#include <16f690.h>
#fuses INTRC_IO, NOWDT, NOMCLR
#USE delay(clock = 4M)
#use spi(MODE=0, DO=PIN_C2, CLK=PIN_C1, ENABLE=PIN_C0, BITS=16)
#use rs232(baud=9600, UART1, ERRORS) |
We are going to run this code in MPLAB vs. 8.92 and run it in the
simulator. So we compile and run and we get this in the Output Window:
And that is your complaint.
Next, we notice (or we already noticed) that you are using int1 variables.
What happens if you left-shift an int1 variable in CCS ? Well, it apparently
shifts the bit right out of existence, because the variable is only an int1.
How do you perform operations on variables when they are of mixed
types ? You promote the smaller variable up to the same size as the
other variable. Sometimes compilers will do this automatically.
Do not assume CCS will do this.
So we cast the int1 up to an int16:
| Code: | | spi_word = (data | ((int16)dac << 15)); |
Now when we run the code we get this:
What you need to do in the future is to aggressively attack the problem
in same way that I did. Be suspicious. Write little tests to verify small
parts of your code. Have the confidence to do this. |
|
|
rikotech8
Joined: 10 Dec 2011
Posts: 376
Location: Sofiq,Bulgariq
|
|
| Posted: Wed Jan 21, 2015 4:02 am |
|
|
Thank you PCM programmer for your time.
The problem still exists with the cast of the int1.
The variable spi_word is still 0 this way:
| Code: | | spi_word = ((int16)dac << 15); |
The only approach that solves my problem is to cast twice like this:
| Code: | | spi_word = ((int16)((int16)dac << 15)); |
Now I get the desired value of 32768.
It is kind of strange for me
Anyway, thank for your advises, I will keep it in mind.
_________________
A person who never made a mistake never tried anything new. |
|
|
rikotech8
Joined: 10 Dec 2011
Posts: 376
Location: Sofiq,Bulgariq
|
|
| Posted: Wed Jan 21, 2015 4:09 am |
|
|
Oh I forgot to post my working code in case someone find this useful.
| Code: |
#include <16f684.h>
#fuses INTRC_IO, NOWDT, NOMCLR
#USE delay(clock = 8M)
#use spi(MODE=0, DO=PIN_C2, CLK=PIN_C1, ENABLE=PIN_C0, BITS=16)
#use rs232(baud=9600, xmit=PIN_A0)
#define DAC_A 0
#define DAC_B 1
#define GAIN_1 1
#define GAIN_2 0
#define OUT_ENABLE 1
#define OUT_DISABLE 0
const unsigned int16 arr[] = {
2048,2151,2255,2358,2460,2561,2660,2759,2855,2949,
3041,3131,3217,3301,3381,3458,3532,3601,3667,3728,
3785,3838,3886,3929,3968,4001,4030,4053,4071,4084,
4092,4095,4092,4084,4071,4053,4030,4001,3968,3929,
3886,3838,3785,3728,3667,3601,3532,3458,3381,3301,
3217,3131,3041,2949,2855,2759,2660,2561,2460,2358,
2255,2151,2048,1944,1840,1737,1635,1534,1435,1336,
1240,1146,1054,964,878,794,714,637,563,494,
428,367,310,257,209,166,127,94,65,42,
24,11,3,0,3,11,24,42,65,94,
127,166,209,257,310,367,428,494,563,637,
714,794,878,964,1054,1146,1240,1336,1435,1534,
1635,1737,1840,1944,2048};
void chip_init();
void spi_data_write(unsigned int16 data, int1 dac, int1 gain, int1 output_enable);
void main()
{
unsigned int i;
chip_init();
while(TRUE)
{
for(i = 0; i < 124; i++)
{
spi_data_write(arr[i], DAC_B, GAIN_2, OUT_ENABLE);
}
}
}
void chip_init()
{
setup_oscillator(OSC_8MHZ);
}
void spi_data_write(unsigned int16 data, int1 dac, int1 gain, int1 output_enable)
{
unsigned int16 spi_word = 0;
spi_word = (((int16)((int16)dac<<15)) | ((int16)((int16)gain<<13)) | ((int16)((int16)output_enable<<12)) | data );
spi_xfer(spi_word);
} |
For the array calculation I have used the online sine wave array calculator.
I find this page very useful:
http://www.daycounter.com/Calculators/Sine-Generator-Calculator.phtml
_________________
A person who never made a mistake never tried anything new. |
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19369
|
|
| Posted: Wed Jan 21, 2015 9:58 am |
|
|
I'd actually suggest a completely different approach. Use a union:
| Code: |
struct assembly
{
int8 datal;
int8 datah:4;
int8 OE:1;
int8 gain:1;
int8 nothing:1;
int8 dac:1;
};
void spi_data_write(unsigned int16 data, int1 dac, int1 gain, int1 output_enable)
{
union
{
int16 value;
struct assembly input;
} data_word;
int16 temp;
data_word.input.nothing=0;
data_word.input.dac=dac;
data_word.input.gain=gain;
data_word.input.datal=data & 0xFF;
data_word.input.datah=make8(data,1) & 0xF;
spi_xfer(data_word.value);
}
|
Why better?.
Multiple reasons. First it avoids the major problem that will appear if 'data' is larger than 0xFFF. If this happens the value in the high bits of the output word will be corrupted. With this code it won't.
Then size and speed. It writes the bits directly into the required location in the output word. One bit set, or clear and test, for each bit transferred, instead of dozens of rotations. Under half the size of code.....
Then ease. You have the bits laid out in the structure in the layout you want them. You can 'see' it is right, instead of having to count rotations. |
|
|
rikotech8
Joined: 10 Dec 2011
Posts: 376
Location: Sofiq,Bulgariq
|
|
| Posted: Wed Jan 21, 2015 1:47 pm |
|
|
Thank you Ttelmah. You are as right as always. There is no other place to learn such a priceless information. You are not just troubleshooting my code, you are giving me a know-how.
Again, thank you for your time!
_________________
A person who never made a mistake never tried anything new. |
|
|
Ttelmah
Joined: 11 Mar 2010
Posts: 19369
|
|
| Posted: Thu Jan 22, 2015 2:21 am |
|
|
Apologies for putting the 'temp' variable in there. I was using it to test!... 
Glad you get the idea. I like things like this generally because it gives the compiler the freedom to use short-cuts. It's quite interesting to compare the assembly generated between the 'rotation' approach and the 'union' approach. For assembling the output, I'd probably say that bit fields, and the union will win 95% of the time, and make it easier to document what you are trying to do. Also it 'raises' the idea that what you are sending is a 'packet' for the target device. Also works the other way for retrieving bits from the device. |
|
|
|
|
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
