Hello,
I have a function in C where one of the parameters is BYTE pData.
I'm trying to use this function on the Arduino but it's error when I try to send "0xAA0501", but if I remove the * works.
The error is "Invalid conversion from 'long int' to 'byte'"
The error is "Invalid conversion from 'long int' to 'byte*'"
So, fix it. If you need help, read the sticky at the top of the forum.
Are you literally using the string literal "0xAA0501"? or just putting 0xAA0501? These details really matter, which is way it is best to copy and paste the code.
A byte is 8 bits and you cannot put a 32 bit value into 8 bits.
A byte is 8 bits and you cannot put a 32 bit value into 8 bits.
Uh, note that it was a pointer to a BYTE, not a BYTE.
If he had put quotes around the string, it might have worked, though a cast to BYTE * might have been needed from the char *.
Six hex digits would be a 24-bit value. The 328's address bus is up to 14 bits wide I believe, so I think that would still be invalid even as a pointer address.
I think that Reply #1 is the only one that needs to be considered here.
KeithRB:
Are you literally using the string literal “0xAA0501”? or just putting 0xAA0501? These details really matter, which is way it is best to copy and paste the code.
The “crc16_lsb” was written in C, I’m trying to use the Arduino now.
#include <WinDef.h>
void setup()
{
// Open serial communications and wait for port to open:
Serial.begin(9600);
pinMode(13, OUTPUT);
pinMode(7, OUTPUT);
byte *valor = 0xAA0501;
crc16_lsb(*valor, (word)sizeof(*valor));
}
void loop() // run over and over
{
char c;
if (Serial.available())
c = Serial.read();
if (c == '1') digitalWrite(13, HIGH);
if (c == '0') digitalWrite(13, LOW);
if (c == 'A') digitalWrite(7, HIGH);
if (c == 'F') digitalWrite(7, LOW);
}
word crc16_lsb(byte *pData, word length)
{
byte i;
word data, crc;
crc = 0xFFFF;
if (length == 0)
return 0;
do
{
data = (word)0x00FF & *pData++;
crc = crc ^ data;
for (i = 8; i > 0; i--)
{
if (crc & 0x0001)
crc = (crc >> 1) ^ 0x8408;
else
crc >>= 1;
}
}
while (--length);
crc = ~crc;
Serial.println(crc, HEX);
return (crc);
}
You are all messed up here.
Your function is expecting an array of bytes.
So something like this:
byte data[] = {0xA, 0xA, 0x0, 0x5, 0x0, 0x01};
int size = 6;
// ...
crc_lsb(&data[0], size);
KeithRB:
You are all messed up here.Your function is expecting an array of bytes.
So something like this:
byte data[] = {0xA, 0xA, 0x0, 0x5, 0x0, 0x01};
int size = 6;
// ...
crc_lsb(&data[0], size);
KeithRB,
Don't work, the result should be "D550". =(
But no more compiler error, right?
KeithRB:
But no more compiler error, right?
Correct, compile without error.
Follows function in C that I have already looked several times and have not found where the error may be.
I believe that is one operator that has different function.
WORD crc16_lsb(BYTE *pData, WORD length)
{
BYTE i;
WORD data, crc;
crc = 0xFFFF;
if (length == 0)
return 0;
do
{
data = (WORD)0x00FF & *pData++;
crc = crc ^ data;
for (i = 8; i > 0; i--)
{
if (crc & 0x0001)
crc = (crc >> 1) ^ 0x8408;
else
crc >>= 1;
}
}
while (--length);
crc = ~crc;
return (crc);
}
byte *valor = 0xAA0501;
crc16_lsb(*valor, (word)sizeof(*valor));
Why are using using a pointer to byte where a byte is more reasonable? Lose all the *s. That value won't fit in a byte. It looks like three bytes, to me.
byte valor[] = { 0xAA, 0x05, 0x01 };
oops, good catch Paul, though you are a little late to the party.
My fix should have read:
BYTE data[] = {0xAA, 0x05, 0x01};
WORD dataSize = 3;
//...
Replace "word" with "unsigned int" or better yet "uint16_t" and then you can drop the windef.h file too.
SirNickity:
Replace "word" with "unsigned int" or better yet "uint16_t" and then you can drop the windef.h file too.
Compile without error, but the return value is not correct.
Is returning "BA1D" instead of "D550". =(
Did you make my correction above to evaluate bytes instead of nibbles?
Also, this is a perfect example where you need to "be the computer" sit down with a pencil and paper and work through the code with the various inputs. At some point, you should see where the code diverges from reality.
Try this:
void setup()
{
uint16_t crcval;
// Open serial communications and wait for port to open:
Serial.begin(9600);
pinMode(13, OUTPUT);
pinMode(7, OUTPUT);
// *** Use a statically-defined array of bytes, like this:
char valor[] = { 0xAA, 0x05, 0x01 };
// *** You can divide the size of the array by the size of one array element
// to get the length of an array. (In this case, each element is one byte, so
// you're dividing by one, but it's better to use the correct formula unless
// the useless divide operation is performance-critical in your application.)
crcval = crc16_lsb(valor, sizeof(valor) / sizeof(valor[0]));
}
void loop() // run over and over
{
char c;
if (Serial.available())
c = Serial.read();
if (c == '1') digitalWrite(13, HIGH);
if (c == '0') digitalWrite(13, LOW);
if (c == 'A') digitalWrite(7, HIGH);
if (c == 'F') digitalWrite(7, LOW);
}
// *** Dropping all the word references and using std int types instead
uint16_t crc16_lsb (char *pData, uint16_t length)
{
// *** Using uint8_t instead of byte. They're equivalent, so it's a matter
// of preference, but I believe in using a type that defines the variable's
// purpose -- a byte would signify to me that its contents is probably
// binary, whereas uint8_t just tells me 'i' is an 8-bit unsigned integer.
uint8_t i;
uint16_t data, crc; // *** Again, dropping the word type
crc = 0xFFFF;
// *** I'm removing the "if length == 0" thing and using a loop where
// the while conditional is up top. This skips the loop when length == 0,
// so there's no need to handle that case specifically. The only difference
// in functionality is what gets returned when length == 0. Before, the
// code returned "0x0000". Now, it returns "0xFFFF" -- the crc seed
// value. I think this is actually what's supposed to happen, but if I'm
// wrong, undo that change.
while (length--) {
// *** You need to cast "*pData" to an 8-bit type here to operate on
// one byte at a time, otherwise the compiler will use the default int
// type -- which is 16-bit, since the pointers refers to a 16-bit type.
// You don't, however, need to cast the literal (0x00FF) to a 16-bit
// type. It already is. (32-bit/64-bit computers would be another
// matter though...)
data = 0x00FF & (uint8_t)*pData++;
crc = crc ^ data;
for (i = 8; i > 0; i--)
{
if (crc & 0x0001)
crc = (crc >> 1) ^ 0x8408;
else
crc >>= 1;
}
}
crc = ~crc;
Serial.println(crc, HEX);
return (crc);
}
SirNickity:
Try this:void setup()
{
uint16_t crcval;
// Open serial communications and wait for port to open:
Serial.begin(9600);
pinMode(13, OUTPUT);
pinMode(7, OUTPUT);
// *** Use a statically-defined array of bytes, like this:
char valor = { 0xAA, 0x05, 0x01 };
// *** You can divide the size of the array by the size of one array element
// to get the length of an array. (In this case, each element is one byte, so
// you're dividing by one, but it's better to use the correct formula unless
// the useless divide operation is performance-critical in your application.)
crcval = crc16_lsb(valor, sizeof(valor) / sizeof(valor[0]));
}
void loop() // run over and over
{
char c;
if (Serial.available())
c = Serial.read();
if (c == '1') digitalWrite(13, HIGH);
if (c == '0') digitalWrite(13, LOW);
if (c == 'A') digitalWrite(7, HIGH);
if (c == 'F') digitalWrite(7, LOW);
}
// *** Dropping all the word references and using std int types instead
uint16_t crc16_lsb (char *pData, uint16_t length)
{
// *** Using uint8_t instead of byte. They're equivalent, so it's a matter
// of preference, but I believe in using a type that defines the variable's
// purpose -- a byte would signify to me that its contents is probably
// binary, whereas uint8_t just tells me 'i' is an 8-bit unsigned integer.
uint8_t i;
uint16_t data, crc; // *** Again, dropping the word type
crc = 0xFFFF;
// *** I'm removing the "if length == 0" thing and using a loop where
// the while conditional is up top. This skips the loop when length == 0,
// so there's no need to handle that case specifically. The only difference
// in functionality is what gets returned when length == 0. Before, the
// code returned "0x0000". Now, it returns "0xFFFF" -- the crc seed
// value. I think this is actually what's supposed to happen, but if I'm
// wrong, undo that change.
while (length--) {
// *** You need to cast "*pData" to an 8-bit type here to operate on
// one byte at a time, otherwise the compiler will use the default int
// type -- which is 16-bit, since the pointers refers to a 16-bit type.
// You don't, however, need to cast the literal (0x00FF) to a 16-bit
// type. It already is. (32-bit/64-bit computers would be another
// matter though...)
data = 0x00FF & (uint8_t)*pData++;
crc = crc ^ data;
for (i = 8; i > 0; i--)
{
if (crc & 0x0001)
crc = (crc >> 1) ^ 0x8408;
else
crc >>= 1;
}
}
crc = ~crc;
Serial.println(crc, HEX);
return (crc);
}
XD XD XD XD XD XD XD XD XD XD XD
It was perfect!!
I did some testing and all were ok.
I will now check what has been changed to enteder operation.
Thank you !!!!!
You're welcome. 
All my changes are marked with a comment like this:
// *** This is what I changed and why
There should be enough there to explain what's different, but if you don't understand something, just ask.