From 2eddc1352801aa710338893a948122916c56c074 Mon Sep 17 00:00:00 2001 From: Scott Bezek Date: Sun, 10 Sep 2023 17:25:23 -0700 Subject: [PATCH] Add a bunch of proto documentation --- firmware/src/proto_gen/smartknob.pb.h | 102 ++++++++++++++++++++- proto/smartknob.proto | 125 +++++++++++++++++++++++++- 2 files changed, 225 insertions(+), 2 deletions(-) diff --git a/firmware/src/proto_gen/smartknob.pb.h b/firmware/src/proto_gen/smartknob.pb.h index dbed108..c847e22 100644 --- a/firmware/src/proto_gen/smartknob.pb.h +++ b/firmware/src/proto_gen/smartknob.pb.h @@ -10,6 +10,7 @@ #endif /* Struct definitions */ +/* * Lets the host know that a ToSmartknob message was received and should not be retried. */ typedef struct _PB_Ack { uint32_t nonce; } PB_Ack; @@ -19,7 +20,26 @@ typedef struct _PB_Log { } PB_Log; typedef struct _PB_SmartKnobConfig { + /* * + Set the integer position. + + Note: in order to make SmartKnobConfig apply idempotently, the current position + will only be set to this value when it changes compared to a previous config (and + NOT compared to the current state!). So by default, if you send a config position + of 5 and the current position is 3, the position may remain at 3 if the config + change to 5 was previously handled. If you need to force a position update, see + position_nonce. */ int32_t position; + /* * + Set the fractional position. Typical range: (-snap_point, snap_point). + + Actual range is technically unbounded, but in practice this value will be compared + against snap_point on the next control loop, so any value beyond the snap_point will + generally result in an integer position change (unless position is already at a + limit). + + Note: idempotency implications noted in the documentation for `position` apply here + as well */ float sub_position_unit; /* * Position is normally only applied when it changes, but sometimes it's desirable @@ -28,15 +48,69 @@ typedef struct _PB_SmartKnobConfig { NOTE: Must be < 256 */ uint8_t position_nonce; + /* * Minimum position allowed. */ int32_t min_position; + /* * + Maximum position allowed. + + If this is the same as min_position, there will only be one allowed position. + + If this is less than min_position, bounds will be disabled. */ int32_t max_position; + /* * The angular "width" of each position/detent, in radians. */ float position_width_radians; + /* * + Strength of detents to apply. Typical range: [0, 1]. + + A value of 0 disables detents. + + Values greater than 1 are not recommended and may lead to unstable behavior. */ float detent_strength_unit; + /* * + Strength of endstop torque to apply at min/max bounds. Typical range: [0, 1]. + + A value of 0 disables endstop torque, but does not make position unbounded, meaning + the knob will not try to return to the valid region. For unbounded rotation, use + min_position and max_position. + + Values greater than 1 are not recommended and may lead to unstable behavior. */ float endstop_strength_unit; + /* * + Fractional (sub-position) threshold where the position will increment/decrement. + Typical range: (0.5, 1.5). + + This defines how hysteresis is applied to positions, which is why values > */ float snap_point; + /* * + Arbitrary 50-byte string representing this "config". This can be used to identify major + config/mode changes. The value will be echoed back to the host via a future State's + embedded config field so the host can use this value to determine the mode that was + in effect at the time of the State snapshot instead of having to infer it from the + other config fields. */ char text[51]; + /* * + For a "magnetic" detent mode - where not all positions should have detents - this + specifies which positions (up to 5) have detents enabled. The knob will feel like it + is "magnetically" attracted to those positions, and will rotate smoothy past all + other positions. + + If you want to have more than 5 magnetic detent positions, you will need to dynamically + update this list as the knob is rotated. A recommended approach is to always send the + _nearest_ 5 detent positions, and send a new Config message whenever the list of + positions nearest the current position (as reported via State messages) changes. + + This approach enables effectively unbounded detent positions while keeping Config + bounded in size, and is resilient against tightly-packed detents with fast rotation + since multiple detent positions can be sent in advance; a full round-trip Config-State + isn't needed between each detent in order to keep up. */ pb_size_t detent_positions_count; int32_t detent_positions[5]; + /* * + Advanced feature for shifting the defined snap_point away from the center (position 0) + for implementing asymmetric detents. Typical value: 0 (symmetric detent force). + + This can be used to create detents that will hold the position when carefully released, + but can be easily disturbed to return "home" towards position 0. */ float snap_point_bias; /* * Hue (0-255) for all 8 ring LEDs, if supported. Note: this will likely be replaced @@ -45,11 +119,37 @@ typedef struct _PB_SmartKnobConfig { } PB_SmartKnobConfig; typedef struct _PB_SmartKnobState { + /* * Current integer position of the knob. (Detent resolution is at integer positions) */ int32_t current_position; + /* * + Current fractional position. Typically will only range from (-snap_point, snap_point) + since further rotation will result in the integer position changing, but may exceed + those values if snap_point_bias is non-zero, or if the knob is at a bound. When the + knob is at a bound, this value can grow endlessly as the knob is rotated further past + the bound. + + When visualizing sub_position_unit, you will likely want to apply a rubber-band easing + function past the bounds; a sublinear relationship will help suggest that a bound has + been reached. */ float sub_position_unit; + /* * + Current SmartKnobConfig in effect at the time of this State snapshot. + + Beware that this config contains position and sub_position_unit values, not to be + confused with the top level current_position and sub_position_unit values in this State + message. The position values in the embedded config message will almost never be useful + to you; you probably want to be reading the top level values from the State message. */ bool has_config; PB_SmartKnobConfig config; - /* * Value that changes each time the knob is pressed */ + /* * + Value that changes each time the knob is pressed. Does not change when a press is released. + + Why this press state a "nonce" rather than a simple boolean representing the current + "pressed" state? It makes the protocol more robust to dropped/lost State messages; if + the knob was pressed/released quickly and State messages happened to be dropped during + that time, the press would be completely lost. Using a nonce allows the host to recognize + that a press has taken place at some point even if the State was lost during the press + itself. Is this overkill? Probably, let's revisit in future protocol versions. */ uint8_t press_nonce; } PB_SmartKnobState; diff --git a/proto/smartknob.proto b/proto/smartknob.proto index a96ee51..0b71058 100644 --- a/proto/smartknob.proto +++ b/proto/smartknob.proto @@ -29,6 +29,7 @@ message FromSmartKnob { } } +/** Lets the host know that a ToSmartknob message was received and should not be retried. */ message Ack { uint32 nonce = 1; } @@ -38,17 +39,70 @@ message Log { } message SmartKnobState { + /** Current integer position of the knob. (Detent resolution is at integer positions) */ int32 current_position = 1; + + /** + * Current fractional position. Typically will only range from (-snap_point, snap_point) + * since further rotation will result in the integer position changing, but may exceed + * those values if snap_point_bias is non-zero, or if the knob is at a bound. When the + * knob is at a bound, this value can grow endlessly as the knob is rotated further past + * the bound. + * + * When visualizing sub_position_unit, you will likely want to apply a rubber-band easing + * function past the bounds; a sublinear relationship will help suggest that a bound has + * been reached. + */ float sub_position_unit = 2; + + /** + * Current SmartKnobConfig in effect at the time of this State snapshot. + * + * Beware that this config contains position and sub_position_unit values, not to be + * confused with the top level current_position and sub_position_unit values in this State + * message. The position values in the embedded config message will almost never be useful + * to you; you probably want to be reading the top level values from the State message. + */ SmartKnobConfig config = 3; - /** Value that changes each time the knob is pressed */ + /** + * Value that changes each time the knob is pressed. Does not change when a press is released. + * + * Why this press state a "nonce" rather than a simple boolean representing the current + * "pressed" state? It makes the protocol more robust to dropped/lost State messages; if + * the knob was pressed/released quickly and State messages happened to be dropped during + * that time, the press would be completely lost. Using a nonce allows the host to recognize + * that a press has taken place at some point even if the State was lost during the press + * itself. Is this overkill? Probably, let's revisit in future protocol versions. + */ uint32 press_nonce = 4 [(nanopb).int_size = IS_8]; } message SmartKnobConfig { + /** + * Set the integer position. + * + * Note: in order to make SmartKnobConfig apply idempotently, the current position + * will only be set to this value when it changes compared to a previous config (and + * NOT compared to the current state!). So by default, if you send a config position + * of 5 and the current position is 3, the position may remain at 3 if the config + * change to 5 was previously handled. If you need to force a position update, see + * position_nonce. + */ int32 position = 1; + + /** + * Set the fractional position. Typical range: (-snap_point, snap_point). + * + * Actual range is technically unbounded, but in practice this value will be compared + * against snap_point on the next control loop, so any value beyond the snap_point will + * generally result in an integer position change (unless position is already at a + * limit). + * + * Note: idempotency implications noted in the documentation for `position` apply here + * as well + */ float sub_position_unit = 2; /** @@ -60,14 +114,83 @@ message SmartKnobConfig { */ uint32 position_nonce = 3 [(nanopb).int_size = IS_8]; + /** Minimum position allowed. */ int32 min_position = 4; + + /** + * Maximum position allowed. + * + * If this is the same as min_position, there will only be one allowed position. + * + * If this is less than min_position, bounds will be disabled. + */ int32 max_position = 5; + + /** The angular "width" of each position/detent, in radians. */ float position_width_radians = 6; + + /** + * Strength of detents to apply. Typical range: [0, 1]. + * + * A value of 0 disables detents. + * + * Values greater than 1 are not recommended and may lead to unstable behavior. + */ float detent_strength_unit = 7; + + /** + * Strength of endstop torque to apply at min/max bounds. Typical range: [0, 1]. + * + * A value of 0 disables endstop torque, but does not make position unbounded, meaning + * the knob will not try to return to the valid region. For unbounded rotation, use + * min_position and max_position. + * + * Values greater than 1 are not recommended and may lead to unstable behavior. + */ float endstop_strength_unit = 8; + + /** + * Fractional (sub-position) threshold where the position will increment/decrement. + * Typical range: (0.5, 1.5). + * + * This defines how hysteresis is applied to positions, which is why values > + */ float snap_point = 9; + + /** + * Arbitrary 50-byte string representing this "config". This can be used to identify major + * config/mode changes. The value will be echoed back to the host via a future State's + * embedded config field so the host can use this value to determine the mode that was + * in effect at the time of the State snapshot instead of having to infer it from the + * other config fields. + */ string text = 10 [(nanopb).max_length = 50]; + + /** + * For a "magnetic" detent mode - where not all positions should have detents - this + * specifies which positions (up to 5) have detents enabled. The knob will feel like it + * is "magnetically" attracted to those positions, and will rotate smoothy past all + * other positions. + * + * If you want to have more than 5 magnetic detent positions, you will need to dynamically + * update this list as the knob is rotated. A recommended approach is to always send the + * _nearest_ 5 detent positions, and send a new Config message whenever the list of + * positions nearest the current position (as reported via State messages) changes. + * + * This approach enables effectively unbounded detent positions while keeping Config + * bounded in size, and is resilient against tightly-packed detents with fast rotation + * since multiple detent positions can be sent in advance; a full round-trip Config-State + * isn't needed between each detent in order to keep up. + */ repeated int32 detent_positions = 11 [(nanopb).max_count = 5]; + + /** + * Advanced feature for shifting the defined snap_point away from the center (position 0) + * for implementing asymmetric detents. Typical value: 0 (symmetric detent force). + * + * This can be used to create detents that will hold the position when carefully released, + * but can be easily disturbed to return "home" towards position 0. + */ float snap_point_bias = 12; /**