Decawave / mynewt-dw1000-core

[DEPRECATED] Use https://github.com/Decawave/uwb-core for new designs.
Apache License 2.0
57 stars 34 forks source link

read/write from/to sub-address 128? #7

Closed 5frank closed 5 years ago

5frank commented 5 years ago

Unless I am mistaken, some read and write operations might fail if sub-address is 128.

The following code (or similar) appears on multiple places:

    dw1000_cmd_t cmd = {
        .reg = reg,
        .subindex = subaddress != 0,
        .operation = 0, //Read or write
        .extended = subaddress > 128,
        .subaddress = subaddress
    };

    uint8_t header[] = {
        [0] = cmd.operation << 7 | cmd.subindex << 6 | cmd.reg,
        [1] = cmd.extended << 7 | (uint8_t) (subaddress),
        [2] = (uint8_t) (subaddress >> 7)
    };

    uint8_t len = cmd.subaddress?(cmd.extended?3:2):1;

if sub-address is 128 the extended bit is (accidentally?) set correctly due to high value an no bit mask, but only the two first header bytes are sent. eg. header sent is [0xNN, 0x80] instead of [0xNN, 0x80, 0x01].

pkettle commented 5 years ago

Hi frank, Good catch,

I propose changing to 0x7F to be consistent with the user manual figure 4.

  dw1000_cmd_t cmd = {
        .reg = reg,
        .subindex = subaddress != 0,
        .operation = 0, //Read or wite
        .extended = subaddress > 0x7F,
        .subaddress = subaddress
    };
5frank commented 5 years ago

Yes that should fix it! I do not understand the purpose of the intermediate cmdstruct, but that is perhaps another topic.

pkettle commented 5 years ago

In C the bitfield placement is undefined, so there is not always a one to one mapping of bitfields to registers bits. The cmd structure captures all the conditional logic into a central location, this is useful for a pipelined architecture.

5frank commented 5 years ago

Ok, thanks!

As I see it, the cmd struct do not provide any abstraction (in the sense that it reduces complexity), so it is like saying the same thing twice. First with a bitfield that can not be used for anything and then with bitmasks, which is the data actually sent.

Please feel free to ignore this discussion that is both off topic and more philosophical by now! :)