Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ToDo: Add Message Handling > 255 (starting with 3) #12

Open
lukebelz opened this issue Jul 24, 2017 · 10 comments
Open

ToDo: Add Message Handling > 255 (starting with 3) #12

lukebelz opened this issue Jul 24, 2017 · 10 comments

Comments

@lukebelz
Copy link

Hi,

First off. Thank you so much for this code. A huge help!

On line 49 of VescUart.cpp, you put //ToDo: Add Message Handling > 255 (starting with 3). I'm trying to read the mcconfig, which returns the starting byte of 3 (since the payload is > 255). As a result, my code goes into case 3, which has this todo marker, and fails.

I've been trying for the last few days to complete this code using all of the documentation given by you and vedder about the communication. With a simple get firmware command, I can see exactly what's going on (I get 2, 3, 0, 2, 18, 84, 17, 3), but with the mcconfig, I get (3, 1, 26, 1, 0, ect... 3). My understanding is, you have start byte of 2, so short packet, packet length of 3, so next 3 bytes for the packet body, then 84 and 17 are the checksum, and finally, 3 is the stop byte. I don't see this pattern because I get the 3 is the start byte, so we should have a long packet body with length 1 + 26 so 27. Not over 255, so why would it be a 3?

Could you either assist me in completing this part of the code or complete it?
Quite a few people are using your code and it's a bummer that it can't do a simple get mc config command.

Thank you

Luke

@RollingGecko
Copy link
Owner

Hi,

I got your problem. In case > 255 you must read byte 2 and 3. Take a look to vedders packet.c:
`void packet_send_packet(unsigned char *data, unsigned int len, int handler_num) {
if (len > PACKET_MAX_PL_LEN) {
return;
}

int b_ind = 0;

if (len <= 256) {
	handler_states[handler_num].tx_buffer[b_ind++] = 2;
	handler_states[handler_num].tx_buffer[b_ind++] = len;
} else {
	handler_states[handler_num].tx_buffer[b_ind++] = 3;
	handler_states[handler_num].tx_buffer[b_ind++] = len >> 8;
	handler_states[handler_num].tx_buffer[b_ind++] = len & 0xFF

...;`

Revers it and you got it. That is the pattern. Waiting for your pull request. :)

Andy

@RollingGecko
Copy link
Owner

Maybe this will help you:

https://en.wikipedia.org/wiki/Bitwise_operation

I also have to get back into this topic first.
Vedder published his explanation after I finished this library. Had a couple of sleepless nights.

@lukebelz
Copy link
Author

lukebelz commented Jul 25, 2017

Thank you! I will take another look at it tomorrow. I tried to go through it all day today, and I learned a lot. Might need a few more days to really understand this code as a whole better.

I see why I need byte 2 and 3, but how do they relate to each other? I take bit 2, shift it back to the left 8, then with bit 3, I don't see how I can go back since your ANDing, your losing data you can't get back... So I'm a bit confused, sorry.

BTW, I not using this library directly. I'm using code from https://github.com/gpxlBen/VESC_Logger/
Which uses some of your code, including what I have mentioned here. But I'll be glad to put a pull request in once I have it working.

@lukebelz
Copy link
Author

So I messed with it some more, And I think at least this part is correct, but it looks like other stuff needs to be modified as well. (using VescUart.cpp for line number references)

Can you think of any other way it would be different? I replace line 40 to 55 with 2 if statements.

  • If count is 2 && payload byte is 2, then it did what it did before.
  • If count is 3 && payload byte is 3, then since we now have byte 2 and 3, not just 2, we take byte 2 and shift back to the left 8 (which gives 256) and add the value of byte 3 to that result. That gives me the payload length, and add 6 (not 5, since we have an extra length byte), and you get the end message.

All in all, it gets to the end of the packet correctly (I hit the end of message loop at line 61), and printing byte by byte, the payload length is correct. But at 255, I get a message not read, and it jumps the counter back to 1. Is it sending multiple packets when the data is over 255 bytes? It seems like something else is different also.

@RollingGecko
Copy link
Owner

Can you post me a message with 255 byte? Beginning and end....

@lukebelz
Copy link
Author

The COMM_FW_VERSION and COMM_GET_APPCONF are good, but the COMM_GET_MCCONF is not. Logged what seemed most useful.

COMM_FW_VERSION.txt
COMM_GET_APPCONF.txt
COMM_GET_MCCONF.txt

@RollingGecko
Copy link
Owner

To be honest, I've no idea. It seams somehow, that the message is wrong and coming that way from the VESC.

@lukebelz
Copy link
Author

If you look at https://github.com/vedderb/bldc/blob/master/packet.c, on line 92, it's similar to the function you wrote. But it is quite different also. Theres many states, not just one like what you have.

@RollingGecko
Copy link
Owner

RollingGecko commented Nov 26, 2017

I did not worked on that subject. Did you had any progress?

@lukebelz
Copy link
Author

I was able to solve the problem, but it requires quite a rewrite. Have not tested it for sending large packets. But I can now get an MCConfig properly. Will update you when I have ore working.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants