btsimonh / hoverboard-firmware-hack

New Hoverboard Firmware Hack. Now written from scratch and generally much better.
GNU General Public License v3.0
22 stars 4 forks source link

Comments on protocol proposal #4

Closed btsimonh closed 5 years ago

btsimonh commented 5 years ago

Comments phail:

btsimonh commented 5 years ago

1/ yes, just to differentiate easily, so you know which CI to immediately look for, and to divert to different processing routines. But unnecessary with (2) below.

2/ I agree, just ACK/NACK to carry CI of message being acknowledged. Every message to get ACK. Response as new message with originator's CI. More messages, but simpler to understand (and only more messages if lots of requests for data).

3/ CRC is quit a lot heavier for a small MCU. CI is normally enough in these circumstances. Hate CRC routines :).

I've updated the wiki reflecting the above, and:

btsimonh commented 5 years ago

code for review - builds but not tested. https://github.com/btsimonh/hoverboard-firmware-hack/tree/newprotocol

p-h-a-i-l commented 5 years ago

I think we are at a good point now. I agree, implementing CRC is annoying and more resource hungry than a simple sum.

What I'd like to have would be a subscription to specific values. Like "Send me the speed each 100ms". But in order to keep it simple just implementing a command would be better. Or maybe this could be the command?

Also I want to trim down some frequently sent commands. The speed data struct is not exactly lightweight with ~60 byte :)

btsimonh commented 5 years ago

ye; not a lot of thought went into the specific commands - i've not even used them yet. I did the generic 'get something' as examples really. I see no reason why a command should not be a request for regular messages.... i.e. to make a subscription. Was thinking that a 2 byte command strategy would work - e.g. cmd = 'S'-> subscribe, next byte 'S' -> speed data, next two bytes -> short ms ? That way we don't run out of meaningful 'primary' commands quickly, and could even have 'P'-> @p-h-a-i-l specials.

btsimonh commented 5 years ago

and rather than cmds 0x80+ from hoverboard, could simplify to 'lower case' from hoverboard, 'upper case' from controller, as a general thing.

p-h-a-i-l commented 5 years ago

Oh yeah, thats a nice Idea! I think my needs are not specific enough to qualify for an own identifier. Yet.

Why do you want to distinguish between hoverboard and controller? On a bus with only 2 end points, it should be always clear!?

btsimonh commented 5 years ago

it just feels right :). But yes, it's not necessary. ref 'P', it means you can claim it for dev use, and then not worry that my or @EmerickH commits will conflict until you are ready to make them 'official' messages which we can all use...

p-h-a-i-l commented 5 years ago

In the protocol we do not describe the existence of the human protocol yet. Do you want to handle it separately? Regarding the human protocol, I think its necessary to have some kind of activation sequence. For example, if you are sending in the machine protocol and the SOM is disturbed for whatever reason, all other characters are parsed as ASCII commands to the human protocol.

btsimonh commented 5 years ago

@p-h-a-i-l - nice PR :). p.s. next step: https://github.com/purwar2016/DeadReckoning-library/wiki/Documentation

add this and we can calculate estimated x/y posn on board. not sure if some thought could go in: because we have the hall sensor interrupts, we have fairly exact timing of encoder change, so theoretically very good wheel position accuracy; but the accurate wheel data is asynchronous, so not sure how the calculation would change. Was considering two optical mice mounted against the inside of the wheels for accurate 'between hall' readings, but how you would coordinate them (from the pi) with the hall position, i'm not sure.

p-h-a-i-l commented 5 years ago

Isn't our resolution from the hall sensors already extremely high?

In a minimal bldc motor 3 Winding, 1 pole pair setup you get hall 6 events per revolution. (mechanical = electrical revolution)

Assumption through quick google: https://vesc-project.com/node/366 27 Stator Poles and 15 pole pairs (30 Magnets) In our setup it should be 27/3 15 6 = 810 hall events per revolution. (1 mechanical revolution = 135 electrical revolutions) The big wheels (10 inch) do have a circumference of around 800mm, so one event per mm driven!?

btsimonh commented 5 years ago

hmm... my (manual) measurement gave me 60 hall changes per rev. -> (6.525.43.142)/60 = ~8.6mm per hall 'tick'. What I don't know is what happens with the bigger wheels - do they have more magnets, or just larger ones :) ? If you have yours running on the bench, it's easy to double check - just display the hall count through debug, and turn the wheel one revolution.

EmerickH commented 5 years ago

Just thinked about this: why not implementing ROS protocol ? http://wiki.ros.org/rosserial It's an already built protocol made for theses kinds of projects, with a large compatibility and there's a STM32 library already made ! What do you think about this?

btsimonh commented 5 years ago

I did think about ROS. But it's a lot more complex (e.g. has md5 in there sometimes!), yet has no continuity indicator. I also considered some other serial ROS protocols, but they were all lacking in some way.