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

MQTT settings/status/telemetry framework #149

Closed
jordens opened this issue Oct 13, 2020 · 1 comment · Fixed by #341
Closed

MQTT settings/status/telemetry framework #149

jordens opened this issue Oct 13, 2020 · 1 comment · Fixed by #341
Labels
enhancement New feature or request

Comments

@jordens
Copy link
Member

jordens commented Oct 13, 2020

To make MQTT easy to deploy on Stabilizer (and Thermostat, Thermostat_EEM, Driver, Booster, Humpback etc.) we need to reduce it's footprint in the code by developing some kind of framework.

Status quo

For Stabilizer we have currently to route the json-over-TCP requests:

stabilizer/src/main.rs

Lines 109 to 158 in 66c917b

macro_rules! route_request {
($request:ident,
readable_attributes: [$($read_attribute:tt: $getter:tt),*],
modifiable_attributes: [$($write_attribute:tt: $TYPE:ty, $setter:tt),*]) => {
match $request.req {
server::AccessRequest::Read => {
match $request.attribute {
$(
$read_attribute => {
let value = match $getter() {
Ok(data) => data,
Err(_) => return server::Response::error($request.attribute,
"Failed to read attribute"),
};
let encoded_data: String<U256> = match serde_json_core::to_string(&value) {
Ok(data) => data,
Err(_) => return server::Response::error($request.attribute,
"Failed to encode attribute value"),
};
server::Response::success($request.attribute, &encoded_data)
},
)*
_ => server::Response::error($request.attribute, "Unknown attribute")
}
},
server::AccessRequest::Write => {
match $request.attribute {
$(
$write_attribute => {
let new_value = match serde_json_core::from_str::<$TYPE>(&$request.value) {
Ok(data) => data,
Err(_) => return server::Response::error($request.attribute,
"Failed to decode value"),
};
match $setter(new_value) {
Ok(_) => server::Response::success($request.attribute, &$request.value),
Err(_) => server::Response::error($request.attribute,
"Failed to set attribute"),
}
}
)*
_ => server::Response::error($request.attribute, "Unknown attribute")
}
}
}
}
}

For Booster there is a sizeable amount of request routing, deserialization, serialization code. Adding a setting or telemetry quantity to the stack requires multiple adjustment in several different places.

Ideas

(ripped from #147): Stabilizer is somewhat similar to redpid (https://github.com/quartiq/redpid). Redpid has a pretty convenient solution to the settings/status problem. For migen/misoc we have a very generic way of configuration-status-registers. They are instantiated in code with a single line (e.g. https://github.com/quartiq/redpid/blob/master/gateware/limit.py#L55) and then end up on a bus with mapping generated automatically (https://github.com/quartiq/redpid/blob/master/test/csrmap.py#L109).
This is very different from Stabilizer in terms of code (HDL versus machine) and transport/protocol (HDL bus CSRs over other buses and serialization versus MQTT) but the user-facing concept is the same: lots of knobs and displays on a big switchboard. The knobs being connected to settings and the displays being connected to telemetry.
Maybe for Stabilizer we want something similar (a named map of settings and status values).
The settings aspect should be double-buffered/paged/ping-pong to allow atomic updates of multiple settings synchronously without having to transfer the entire settings space.
And we need to think a bit about the status telemetry: There might be a case for configurable rates and suppression of some telemetry updates.

Context

@jordens jordens added the enhancement New feature or request label Oct 13, 2020
@irwineffect
Copy link
Collaborator

irwineffect commented Oct 19, 2020

Ryan and I discussed a few ideas yesterday, below are my notes:

Introduction

We are thinking about having a primary setting struct which would be a struct of structs of
fields that operates as a staging area for new settings. The user will define
this structure. Settings change requests from MQTT will operate on this struct,
but the settings will not "go live" until the user initiates a "commit"
request. At this point, the settings framework will trigger an "commit
settings" event. This "update settings" routine would be written by the user, it
will be given the settings structure, the user would use this structure to
populate the individual configurable resources that each task needs.

The Global Setting Structure

This structure would be user-defined. The settings framework would be given an
instance of this struct to store the "staged" settings. We'd like to explore
creating some sort of #derive macro that would let the user define the struct,
and the macro would generate the code needed for the framework to be able to
update the struct. If desired, we'd like to explore a method for the user to be
able to define valid ranges for settings values that would provide automatic
range checking.

Commit Settings

The "commit settings" routine would be responsible for populating the resources
needed by each algorithm. When the user requests settings to be committed, this
routine would be called and given the global setting structure. For example, a
PID controller may have a resource storing the P, I, and D values, and
the update routine would simply need to copy those values from the global
setting structure to the resource. However, in some cases more complex
operations might need to be done, such as performing calculations to derive the
proper values to put in a resource based on the settings. Or, subroutines may
need to be called to configure hardware. This "commit settings" routine would
provide for that flexibility. Additionally, it allows settings updates to be
as atomic as needed with respect to other operations on the device simply by
acquiring locks to the RTIC resources.

MQTT Updates

Still need to figure out the best way to process MQTT requests to update the
settings structure. Suggested solutions:

  1. MQTT requests will be composed of a value and an offset into the settings
    structure. The framework will blindly write this value to the offset in the
    setting structure.
  2. MQTT requests will be composed of a value and a settings identifier of some
    sort (e.g. hierarchical pathname, or a unique ID). The framework will need to
    processes the identifier to determine where in the setting structure to apply
    the value.

The advantage of proposal 1 is that settings updates will be very fast and
require minimal processing on the device. The offsets need to be known to the
configuration software somehow, the most robust system would be for the device
to publish a manifest which maps settings to offset values that the configuration
software uses. This proposal requires the configuration software working
properly to avoid corruption of the settings, a bad offset value could cause
unrelated settings to get modified. Debugging these sorts of issues could
prove challenging. Additionally, because the framework would be blindly
writing values to offsets, no sort of error/range checking would be possible.

Proposal 2 seeks to address the limitations of proposal 1, at the expense of
increased processing responsibility on the device. The device would need to
determine where in the settings structure to write the requested value. This
may be processing intensive, needing much string parsing. However, it opens up
the potential for performing error/range checking.

Another design choice that needs to be evaluated is the granularity of the MQTT
settings requests. Should we have a 1-to-1 mapping of requests to settings
fields, or should it be possible to update multiple settings fields in a single
request, or describe an entire setting structure? Using the simple PID example:

  1. 3 MQTT requests would be required, one for each value.
  2. 1 MQTT request, composed of 3 individual field update requests
  3. 1 MQTT request to update the PID structure, which contains 3 fields

Proposals 1/2 look like they would be simpler to implement, at the expense of
lower throughput if we ever need to do a bulk settings change.

Proposal 3 seeks to correct this issue, but may come at increased complexity,
it still needs to be evaluated. JSON and Serde might make this pretty easy to implement.

Summary

Ideally, the end result would allow us to do roughly something like below:

Creating the settings struct:

// Example Substructure
struct PidSettings {
    p: u32,
    i: u32,
    d: u32,
};
// Primary Settings Structure
#[derive(MqttSettingTrait)]
struct MySettings {
   color: Color,
   pid1: PidSettings,
   pid2: Pidsettings,
}

The Idle task:

// Create an instance of the settings manager
let settings = MqttSettings<MySettings>::new();

// Idle loop
loop {
    // Get messages from MQTT
    let message;
    // Look at the topic prefix to see if we should hand to the settings manager.
    if is_settings_message_topic(message) {
        // Process the settings MQTT request, this will update an internal instance of the MySettings struct
        settings.parse(message);
    }
    // If the message is a request to commit the settings
    if is_commit_message(message) {
        // Schedule the task to commit the settings, pass it a copy of the settings struct
        cx.spawn.commit_settings(settings.get_settings()).unwrap();
    }
}

The commit task:

#[task(resources = [led_driver, pid1_settings, pid2_settings])]
fn commit_settings(cx: commit_settings::Context, new_settings: MySettings) {
    // Use the color setting to change the LED color
    cx.resources.led_driver.lock(|led_driver| {
        // Write to the LEDs
        led_driver.write(new_settings.color);
    });

    // For the PID settings, just copy to the relevant resources
    *cx.resources.pid1_settings = new_settings.pid1;
    // If PID2 used a different structure for it's settings, just drive the From/Into trait
    *cx.resources.pid2_settings = new_settings.pid2.into();
}

External to the device, we publish an MQTT message to the device_identifier/settings/pid1/p topic, then publish to /device_identifier/settings/commit, and the p term of the PID1 setting will be updated, all other settings would maintain their values.

This was referenced Jan 27, 2021
bors bot added a commit that referenced this issue Feb 19, 2021
261: MQTT settings and telemetry r=ryan-summers a=ryan-summers

This PR replaces #242

This PR addresses #149 by exposing a simple MQTT-based settings interface

**TODO**:
- [x] Resolve mutable ownership issues in the `MqttInterface`
- [x] Expand settings interface to `lockin.rs`
- [x] Add all settings (not yet supported by `miniconf`)
- [x] ~~Finalize and publish the `miniconf` package~~ Deferred to the future.
- [x] Move `MqttInterface`-like behaviors over to `miniconf`
- [x] Move smoltcp-nal to a separate crate
- [x] Test on hardware
- [x] ~~Implement stabilizer telemetry~~: Deferred to future

Co-authored-by: Ryan Summers <[email protected]>
Co-authored-by: Robert Jördens <[email protected]>
@bors bors bot closed this as completed in 3354979 May 10, 2021
@bors bors bot closed this as completed in #341 May 10, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants