Skip to content

Plot decoded recorder samples in seconds #95

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.

Sign up for GitHub

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

Open
mjc wants to merge 3 commits into
base: main
Choose a base branch
from
+16 −4
Open

Plot decoded recorder samples in seconds #95

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
24f697a
Recorder: Decode values for plots
mjc Jul 13, 2026
185af2a
Recorder: Hoist plot timestamp conversion
mjc Jul 16, 2026
6420409
Data Recording: Plot relative timestamps
mjc Jul 24, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions src/conf/buffer.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,14 @@ uint16_t to_float16(float x) {
return (b&0x80000000)>>16 | (e>112)*((((e-112)<<10)&0x7C00)|m>>13) | ((e<113)&(e>101))*((((0x007FF000+m)>>(125-e))+1)>>1) | (e>143)*0x7FFF;
}

float from_float16(uint16_t x) {
const float sign = x & 0x8000u ? -1.0f : 1.0f;
const uint16_t exponent = (x >> 10) & 0x1fu;
const uint16_t mantissa = x & 0x03ffu;
return sign * (exponent ? ldexpf(1024.0f + mantissa, exponent - 25)
: ldexpf(mantissa, -24));
}

@lukash lukash Jul 19, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where does this code come from? I'd prefer to use the form from the stack overflow post.

@mjc mjc Jul 20, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I missed that the existing encoder’s cited Stack Overflow answer also included the matching decoder.

This seems to be a small piece of "works on my compiler" archaeology, in the original Stack Overflow answer.

The encoder from the post evaluates an invalid shift when the exponent field is at most 93 or at least 126, including both 0.0f and ordinary values such as 1.0f. The decoder does so whenever the mantissa is zero, including zero and normalized powers of two. Its sign-bit shift also needs to be made unsigned. UBSan catches these, though the current Cortex-M4 output happens to produce the intended results.

@mjc mjc Jul 20, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here’s how to do both, without UB, while retaining branchless output with GCC 13.3.1 and the current Cortex-M4 -Os flags:

#include <string.h>

uint16_t to_float16(float x) {
    uint32_t bits;
    memcpy(&bits, &x, sizeof(bits));

    const uint32_t b = bits + 0x00001000;
    const uint32_t e = (b & 0x7F800000) >> 23;
    const uint32_t m = b & 0x007FFFFF;

    /*
     * C's conditional operator only evaluates the selected side, so
     * (125 - e) is used as a shift count only for 102 <= e <= 112.
     */
    const uint32_t denormalized = (e < 113 && e > 101)
        ? ((((0x007FF000 + m) >> (125 - e)) + 1) >> 1)
        : 0;

    return (b & 0x80000000) >> 16 |
           (e > 112) *
               ((((e - 112) << 10) & 0x7C00) | (m >> 13)) |
           denormalized |
           (e > 143) * 0x7FFF;
}

float from_float16(uint16_t x) {
    const uint32_t e = (x & 0x7C00) >> 10;
    const uint32_t m = (x & 0x03FF) << 13;

    const float mf = (float)m;
    uint32_t mf_bits;
    memcpy(&mf_bits, &mf, sizeof(mf_bits));

    const uint32_t v = mf_bits >> 23;

    /*
     * For m == 0, this produces a safe shift count of zero.
     * For m != 0, 150 - v is already in the valid 1..10 range.
     */
    const uint32_t shift = (150 - v) * (m != 0);

    const uint32_t bits =
        ((uint32_t)(x & 0x8000) << 16) |
        (e != 0) * ((e + 112) << 23 | m) |
        ((e == 0) & (m != 0)) *
            ((v - 37) << 23 |
             ((m << shift) & 0x007FE000));

    float result;
    memcpy(&result, &bits, sizeof(result));
    return result;
}

to_float16 retains the same 104-byte code size, with a different instruction sequence.

from_float16 pays an extra 12 bytes for the safe zero selection. no data-dependent branches in either.

The memcpy calls are the standard defined bit-cast idiom and optimize away with both GCC and Clang.

Clang lowers this conditional to branches. Masking the shift count with & 31 keeps it branchless there too, but grows Clang’s encoder from 104 to 106 bytes and GCC’s from 104 to 108 bytes, so I’ve left the GCC-optimal form here.

original encoder asm: shift unconditionally, discard invalid results after.
revised encoder asm: predicate shift with an IT block.
decoder does moveq to replace the shift count with zero when the mantissa is zero.

I understand if you might not want this, hence posting it as a comment


// clang-format on
#pragma GCC diagnostic pop

Expand Down
1 change: 1 addition & 0 deletions src/conf/buffer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include <stdint.h>

uint16_t to_float16(float x);
float from_float16(uint16_t x);

void buffer_append_int16(uint8_t *buffer, int16_t number, int32_t *index);
void buffer_append_uint16(uint8_t *buffer, uint16_t number, int32_t *index);
Expand Down
11 changes: 7 additions & 4 deletions src/data_recorder.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,12 +131,13 @@ void data_recorder_sample(DataRecord *dr, const Data *d, time_t time) {
}

static void send_point_vt_experiment(const void *item, void *data) {
unused(data);
const time_t start_time = *(const time_t *) data;

Sample *sample = (Sample *) item;
const Sample *sample = (const Sample *) item;
const float time = (sample->time - start_time) * (1.0f / SYSTEM_TICK_RATE_HZ);
for (uint8_t i = 0; i < ITEMS_COUNT_REC(RT_DATA_ALL_ITEMS); ++i) {
VESC_IF->plot_set_graph(i);
VESC_IF->plot_send_points(sample->time, sample->values[i]);
VESC_IF->plot_send_points(time, from_float16(sample->values[i]));
Comment thread
mjc marked this conversation as resolved.
}
}

Expand All @@ -151,7 +152,9 @@ void data_recorder_send_experiment_plot(DataRecord *dr) {
VISIT_REC(RT_DATA_ALL_ITEMS, ADD_GRAPH);
#undef ADD_GRAPH

circular_buffer_iterate(&dr->buffer, &send_point_vt_experiment, 0);
Sample first_sample = {0};
circular_buffer_get(&dr->buffer, 0, &first_sample);
circular_buffer_iterate(&dr->buffer, &send_point_vt_experiment, &first_sample.time);
}

typedef enum {
Expand Down
Loading