What about the first byte there...

Question

5.00/5 (1 vote)

See more:

Here my code...

C++

void rfid_msg(byte opcode, byte* data, int len)
{
    byte* msg = new byte[len + 5];
    byte response[256];

    msg[0] = 0xff;
    msg[1] = len;
    msg[2] = opcode;

    if (len > 0) {
        memcpy(&msg[3], data, len);
    }

    unsigned int tmp = crc(&msg[1], len + 2);

    msg[len + 3] = tmp >> 8;
    msg[len + 4] = tmp & 0xff;

    Serial.write(msg, len + 5);
}

rfid_msg(0x0c, new byte[0], 0);

For this specific 'message' the data to send should be a five byte array with the values 0xff, 0x00, 0x0c, 0x1d, 0x03 (the crc computes that 0x1d, 0x03 thing)...
However the first (and only the first) byte comes as 0x16 - always...

If I change the this line:

C++

byte* msg = new byte[len + 5];

to this:

C++

byte msg[] = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0};

(16 bytes is enough for sure)
than everything works perfectly and the first byte comes as 0xff...

Any explanation?

Posted 16-Nov-15 10:14am

Kornfeld Eliyahu Peter

Add a Solution

Comments

Afzaal Ahmad Zeeshan 16-Nov-15 16:22pm

5ed; for a very interesting question there.

I would also be interested in looking forward to answer to this problem.

[no name] 16-Nov-15 18:26pm

What am I missing?
0xff + opcode + len + crc == 6 bytes
byte* msg = new byte[len + 5]; // does not allocate enough

2 solutions

Add a Solution

Add your solution here

Treat my content as plain text, not as HTML

Preview 0

…

Existing Members

Sign in to your account

...or Join us

Download, Vote, Comment, Publish.

Your Email
Password
Forgot your password?

Your Email
This email is in use. Do you need your password?
Optional Password

I have read and agree to the Terms of Service and Privacy Policy
Please subscribe me to the CodeProject newsletters

When answering a question please:

Read the question carefully.
Understand that English isn't everyone's first language so be lenient of bad spelling and grammar.
If a question is poorly phrased then either ask for clarification, ignore it, or edit the question and fix the problem. Insults are not welcome.
Don't tell someone to read the manual. Chances are they have and don't get it. Provide an answer or move on to the next question.

Let's work to help developers, not make them feel stupid.

This content, along with any associated source code and files, is licensed under The Code Project Open License (CPOL)

User 59241 · Answer 1 · 2015-11-16T14:37:00

Solution 1

Hard coded constants are dangerous.
https://www.arduino.cc/en/Reference/Int[^]
0xff + opcode + len + crc == 6 bytes

C++

void rfid_msg(byte opcode, byte* data, int len)
{
    byte* msg = new byte[len + 6];
    byte response[256];
 
    msg[0] = 0xff;
    msg[1] = len >> 8;
    msg[2] = len & 0xff;
    msg[3] = opcode;
 
    if (len > 0) {
        memcpy(&msg[4], data, len);
    }
 
    unsigned int tmp = crc(&msg[1], len + 3);
 
    msg[len + 4] = tmp >> 8;
    msg[len + 5] = tmp & 0xff;
 
    Serial.write(msg, len + 6);

    delete [] msg;
}

Posted 16-Nov-15 14:37pm

User 59241

Updated 16-Nov-15 15:31pm

v5

Comments

George Jonsson 16-Nov-15 21:05pm

Will this line really work?
msg[1] = len;
Shouldn't the integer be split up in two bytes.
I haven't programmed C++ in a long time, but I think you will only get the truncated value and the byte msg[2] will be empty.

[no name] 16-Nov-15 21:07pm

Correct same as crc. Code reviews do work.

George Jonsson 16-Nov-15 21:17pm

They do. :-)

Kornfeld Eliyahu Peter 17-Nov-15 2:09am

You missed some points here...
There is a thing called implicit type casting in C/C++, so when you do a 'byte = int' thin you will not get error or corrupted memory, but the compiler will wrap it in a type cast code...
You also missed the point where I told, that if I declare a fixed size array, and not allocating it run-time (using new) it all works perfect...

Even if you would imagine that msg[1] = len would write a 2 byte value into msg, the next line msg[2] = opcode should overwrite the second byte of len and all should go perfectly...(maybe msg[1] will have some problems depending on the endianess of the platform)

You also missed the point that the only part corrupted is msg[0] and it corrupted only if I use new to allocate the array dynamically...

And given the exact case (opcode= 0x0c, no data) all the theory (even if it was right) do not explains the 0x16 value comes on at msg[0]...

[no name] 17-Nov-15 2:30am

Your response seems a little haughty as I am only trying to help you.
1. Agree with the typecasting - so you want the int truncated. A comment is good.
2. I can't run your code so only visual examination possible.
3. A better test would be to declare an array of 5 bytes and see what happens. If that replicates the error add and extra byte and zero it. If that fixes it then it may be how the data is being read.

Kornfeld Eliyahu Peter 17-Nov-15 2:36am

byte msg = new byte[5];

msg[0] = 0xff;
msg[1] = 0;
msg[2] = opcode;

unsigned int tmp = crc(&msg[1], 3);

msg[4] = tmp >> 8;
msg[5] = tmp & 0xff;

This still comes wrong at msg[0]...but!

byte msg[5] = {0, 0, 0, 0, 0};

works perfectly...

[no name] 17-Nov-15 2:40am

OK I added something which now is not relevant. Puzzling.

Kornfeld Eliyahu Peter 17-Nov-15 2:43am

I'm apologize for the way I addressed your answer in my previous comment...I had no intention to be rude. I'm aware of the good will brought you here, only tried to clarify the situation...
My true apologies...

Kornfeld Eliyahu Peter 17-Nov-15 2:39am

Just to add...I do not think the problem is with the byte array, but around the Serial.write somehow...

[no name] 17-Nov-15 2:41am

Our comments crossed paths - that's what I added but does not explain why your test works.

Kornfeld Eliyahu Peter 17-Nov-15 2:51am

It is a most strange thing, and my suspect is Serial.write...
I will try to use malloc instead of new to see how it comes...
Anyway - thanks for your time...

[no name] 17-Nov-15 2:59am

No problem. I think new is just a wrapper for malloc anyway.

[no name] 17-Nov-15 4:12am

Just to let you know I tested this in a loop with atmega256 and it works as expected with new or declared array.

//byte* msg = new byte[5];
byte msg[5];

msg[0] = 0xff;
msg[1] = 0x00;
msg[2] = 0x0c;
msg[3] = 0x1d;
msg[4] = 0x03;

Serial.write(msg, 5);

delay(2500);

// delete [] msg;

I display the data as hex using my own component from CP. Also I use eclipse.

Kornfeld Eliyahu Peter 17-Nov-15 4:19am

Thank you...
It is really fascinating...
I have an atmega328...

Unfortunately I have to work now - so will play it later...

Patrice T · Answer 2 · 2015-11-16T15:20:00

Solution 2

In order to make sure of what the code is doing, I would fire the debugger on the code.
I think this code is a good example of when it become useful to run the debugger.

http://playground.arduino.cc/Code/VisualMicro[^]
http://www.visualmicro.com/page/Debugging-for-Arduino.aspx[^]
http://playground.arduino.cc/Main/DevelopmentTools[^]
http://stackoverflow.com/questions/7225693/how-do-you-debug-arduino-code-running-on-arduino-hardware[^]

Posted 16-Nov-15 15:20pm

Patrice T

Updated 16-Nov-15 15:46pm

v2

Comments

[no name] 16-Nov-15 21:37pm

No built in debugger in Arduino environment.
http://www.codeproject.com/Articles/1037057/Debugger-for-Arduino

Patrice T 16-Nov-15 21:44pm

Pitiful :(
But quick search gives results, are you sure of this ?

[no name] 16-Nov-15 21:46pm

Yes - having used Arduino for many years I've learnt to live with it - Read the intro in the link I gave.

Kornfeld Eliyahu Peter 17-Nov-15 2:00am

I would like to use a debugger, but even VisualMicro (the best I found and paid for!) not really helps here...
The problem is somewhere around the Serial.write and not with the byte-array...