Add support for Berlinger temperature sensors #1798
Comments
|
Thanks @adrianwb, sorry have quite a few things on at the moment, but I'll try to dedicate a little bit of time per day on this, I looked at
I think that's responsibility of consumer of this crate to store temperature logs and merge breaches, etc..
I thought they were always configurable the user but the title of this page suggests that it's only when Berlinger sets it as configurable from factory
👍
Here 'for a given configuration', and 'apply arbitrary breach configuration', to me this implies this crate would try to deduce temperature breaches (have logic to calculate them), is that the case ? I think that logic should be in omSupply (since we need this logic for temperature data from other sensors).
To populate our breach configurations based on sensor breach configurations ? From 3
Can these breaches be acknowledge by the user on the actual device ? If so, is it provided in the data we can capture from device ? I have a few more questions about sensors and the data they provide, and I am sure I am not the only one. I think API description (of the crate) is the best way to communicate this (and share the knowledge you have of integration with those sensor). Btw I noticed:
And
And I think that a good data structure here would be of union/enum type rather then optional/nullable fields, since this way a structure could represent capabilities of that particular device. i.e. device with raw temperature data will result in structure with temperature logs field and device without logs would return a structure without those field. But i guess this would depend on how the data is queried (all at once or granular, I think all at once would be sufficient for now) From 4
I think this could be a listener provided by the crate, makes it easier for consumer and I think a good separation of concern (since it can also identify that it's not just any USB but the correct device) From 3
Why is it better ? |
I was anticipating for MVP we would just capture RTMD-generated breach events. Calculating (or deducing) breach events based on our own logic was a lower priority and I agree it would be best to do that in omSupply so we can use the same logic for all temperature data sources. On that note, we should store both RTMD-generated breach events and our own system-generated breach events so that we can leverage them / compare them where necessary. |
|
On the note of acknowledging, due to our experience in # it would be good to be able to record where an acknowledgement has taken place so that we can leverage this to do the appropriate action in the appropriate place. E.g.
RTMD being a catch-all (Cold Chain Standalone app + sensors would = an RTMD) |
|
Thanks @andreievg for taking the time to have a look - much appreciated, especially as I try to get up to speed on a lot of new stuff!
Absolutely agree that the consumer of the crate is responsible for storing the data. I was thinking that saving to a locally stored file would be a useful feature to have during development to help with debugging, especially before the crate is integrated into omSupply, but maybe disable that in release versions?
Indeed, that's what I'd initially thought as well, until I discovered that you have to request Berlinger to generate the configuration you want (although it seems that you can update some existing models if you have access to a suitable configuration file).
Depending on the Berlinger sensor model, it will have generated some of its own breaches (just cumulative ones for Fridgetags), so that built-in breach data would be part of the crate. However, you and @adamdewey are right that we need that logic (for analysing temperature log data to update/create breaches for a given breach config) to be available for all sensor data => maybe in another separate crate, so that I can work on it outside of omSupply for now? As @adamdewey mentions in another comment, this extra temperature log analysis logic doesn't have to be in the MVP and can be added later. I'd suggest we add a That also has implications for acknowledgements, as clearly a device won't be able to acknowledge a custom config breach - that'll only be possible in omSupply (or mSupply desktop)
Yes
I think so, although I haven't actually tried it yet. Obviously that would be better, as the device really ought to be the definitive source of truth, although @adamdewey makes the point above that we should consider breach acknowledgements from multiple sources i.e. the device itself, or in the active store (omSupply or desktop mSupply).
Yes, I'll try to design & document the API for the crate as one of the first things.
Yes, that makes sense - thanks.
Yes, that would be ideal.
It seems to me to be quite complicated to try to maintain links in the DB itself between temperature logs and related breaches and vice-versa, as it's a many-to-many relation which depends on the breach configurations. We would have to use some sort of join table, which is a bit messy IMHO, and that would get even more messy if can apply/edit custom breach configs in addition to, or instead of, the pre-defined ones from the sensor. It's a simple enough query to lookup temperature logs for a given breach, or to lookup breaches for a given temperature log, and I can't see any use case where it wouldn't just be a single lookup, so that's why I'm suggesting that's the way to go => keep it flexible rather than get tied up in trying to maintain brittle many-to-many relations (which would break if you edit the breach config). |
Thanks for your patience, in advanced, I'll have a lot of questions to ask.
I was unsure when I saw
Oki, although I think we would need to think about where this logic can be inserted, so that, at the very least you can start working with omSupply repository (even if it's just adding tests to see that breaches are calculated correctly), btw, during cold chain app development I played around with calculating breaches with database: https://github.com/sussol/CCEapp-playground/blob/master/sqlForNormalBreach/README.MD and https://github.com/sussol/CCEapp-playground/blob/master/sqlForNormalBreach/otherQueries.sql (the later is much faster and cleaner, this is for consecutive breaches i am pretty sure, since cumulative breaches seemed pretty easy with sql, it's been awhile thought).
Sounds good (at least at this point in design). Also agree that MVP would just be device breaches.
Also agree that it's a good point, but I do think we need to consider how this would work UX wise @adamdewey, do we always have to double acknowledge breach ? Or acknowledgement on device end is enough ? Or ?
Thank you
I am on board, I think, for not linking temperature logs to breaches (at least for now, not sure how this would work when we record breaches for stock, where stock can move and logs will come from different sensors). But in terms of being brittle, since breaches will be their own entity, how does editing breach configuration affect them ? I think we should snapshot configuration on the breach, and when breach config is edited, we can specify from when it should apply ? And recalculate at this point (keeping existing breaches ?) @adamdewey what do you expect the behaviour should be when breach configuration changes on existing data set ? |
@andreievg We could start with acknowledgement on the RTMD only initially (assuming all RTMDs allow you to acknowledge their own breach events) Multiple acknowledgements would just be for the scenario where you don't want someone unqualified to walk past the RTMD and acknowledge the breach and thus obscure the breach from someone who is responsible for monitoring them remotely (I can see this happening once we add annoying audible alerts). |
@andreievg yes, I agree - it would be good to store a snapshot of the configuration that relates to the breach event if possible. The only reasons I can think of for the user to change the breach configuration would be:
Whichever way it would be a significant change in logistics and not likely to happen often. (There is a 4th reason: they failed to set up the sensor correctly initially, but in this case they wouldn't care about historic breaches.) This actually raises something I was thinking about: making it mandatory for an RTMD to be associated with an omSupply location (and allowing for multiple RTMDs to be associated with one location). Really the user is more concerned about temperature breaches for a location, not a sensor, and would correspondingly be concerned about the breach history for the location, not the sensor per se. TL;DR - As a user I would expected to be able to view what rules were in play at the time a breach was triggered when viewing historic data |
Cool so snapshotting should work, and no recalculation for now, just next breach would take new breach conditions into account. Hmm yeah ideally we would have breach config associated with location (we initially did this for mobile), but it's chicken or the egg thing for cold chain app, since it doesn't know locations (I think we do the location association when it syncs to mSupply). I think this needs to be flagged as something else to discuss, though, what is the ideal way to handle this |
Yeah I think the easiest thing is to consider Cold Chain app + sensors like just any other RTMD (e.g. Berlinger Fridge-tags) and so we shouldn't expect it know anything about locations - we can do all those smarts in omSupply? |
|
Sorry for no recent updates but I have been making progress :-) Here's the basic structure definitions for the crate (in lib.rs): use chrono::{NaiveDateTime, Duration};
#[derive(Debug)]
// define the types encountered when parsing a sensor text file (e.g. Berlinger) => not sure that we need this actually!
pub enum SensorFieldType {
Text(String),
Float(f64),
Integer(i64),
Duration(Duration),
Boolean(bool),
Timestamp(NaiveDateTime),
}
#[derive(Debug)]
// define the four types of breach
pub enum BreachType {
HotConsecutive,
ColdConsecutive,
HotCumulative,
ColdCumulative,
}
#[derive(Debug)]
// define the sensor types supported
pub enum SensorType {
Berlinger,
}
#[derive(Debug)]
// define the structure used to capture a temperature log
pub struct TemperatureLog {
pub temperature: f64,
pub timestamp : NaiveDateTime,
}
#[derive(Debug)]
// define the structure used to capture a breach config
pub struct TemperatureBreachConfig {
pub breach_type: BreachType,
pub maximum_temperature: f64,
pub minimum_temperature: f64,
pub duration: Duration,
}
#[derive(Debug)]
// define the structure used to capture a temperature breach
pub struct TemperatureBreach {
pub breach_type: BreachType,
pub start_timestamp: NaiveDateTime,
pub end_timestamp: NaiveDateTime,
pub duration: Duration,
pub acknowledged: bool,
}
#[derive(Debug)]
// define the structure used to capture sensor details (incomplete)
pub struct Sensor {
pub sensor_type: SensorType,
pub registration: String,
pub name: String,
pub last_connected_timestamp: Option<NaiveDateTime>,
pub breaches: Option<Vec<TemperatureBreach>>,
pub configs: Option<Vec<TemperatureBreachConfig>>,
pub logs: Option<Vec<TemperatureLog>>,
} |
|
And a berlinger.rs which defines the following public functions: pub fn read_sensor_file(file_path: &str) -> Option<Sensor> {}; // read sensor data directly from a Berlinger text file (useful for testing)
pub fn read_sensor_serials() -> Vec<String> (); // return serials of all found sensors
pub fn read_sensor(serial: &str, start_timestamp: Option<NaiveDateTime>, end_timestamp: Option<NaiveDateTime>) -> Option<Sensor> {}; // read sensor data matching given serial, filter temperature logs/breaches between start/end timestamp |
|
Hopefully that's enough to enable someone to start on the UI side, and/or storing/integrating the sensor data into omSupply? I've (hopefully!) nearly finished implementing Later on, I should be able to add the code to process the raw temperature log/breach data and use that to work out arbitrary breaches, but I think we'd decided that was beyond MVP? |
Ohh that's great, so we can have a sample file to work with (locally ?)
Cool, hopefully that would give enough time to figure out omSupply end of it (where to use, what to store it against and how to use it)
Hmm, yeah this might need to be done on omSupply end, to deduce it from temperature logs stored in db, what do you think ?
That looks pretty good thanks
Ohh does the sensor store this ? Btw do we know the temperature capture interval ? |
I'll upload a few samples once I've got it working!
It certainly shouldn't be Berlinger-specific, but I think it could be done in the top level of the crate (lib.rs) - something like: pub fn calculate_breaches(temperature_logs: &Vec<TemperatureLog>, breach_config: &TemperatureBreachConfig) -> Vec<TemperatureBreach> {}
It has a
Yes, there's a |
|
WIP uploaded to https://github.com/openmsupply/temperature-sensor/ - if I've figured it out right, you should have access :-) |
|
@adrianwb, thank you, i made an issue for integrating this crate with omSupply: #1955. I think a few extra change would be needed:
I am not sure if I can get the time to review the code in the next couple of days, but I'll try to look at it eventually. Would be great if you can work on some of omSupply functionality, if you keen let us know I can point you in the right direction for the task that are in this issue (can find example PRs for adding schema and sync etc..) |
|
Yes, a bit of work still to do for proper error handling. I've just updated it to work with QTags (mostly - some extra temperature logs to parse as well, but shouldn't be too hard). And then on to the USB functionality... I'd be happy to have a go at integrating it with the omSupply functionality, but would probably be at least the middle of next week... |
|
The updated version will now read and return FridgeTag and QTag data from any drive/volume less than 8 GB (multiple supported) using https://github.com/ir1keren/rs-drivelist/tree/397ff04842ee6efc77de7a285fd4e8db015f7587 to get the list of drives - it only works on Windows and Linux (== Android??), but there surely must be an equivalent for Mac as well? I'll see if it works in WSL, and if I can find an easy way to build it for Android, I'll try that, but I have almost zero experience of mobile development so probably better to spend some time on the error handling instead! I did look at the USB crates (https://docs.rs/rusb/latest/rusb/) but they seem to be very technical and more about communicating with a USB device -> way more complicated than we need, when essentially all we want to do is to read a file off a USB drive, which is all that desktop mSupply does anyway => detecting USB insertion and triggering it from that seems more bother than it's worth IMHO, at least for the initial stages? |
|
|
|
I just realised that I never commented on the breach config discussions above - in practice the main breach config data is included (albeit indirectly) in the breach itself i.e. the breach type, duration (which will be >= breach duration threshold) and temperature (which will be outside the breach min/max range) => I'm not convinced that it's necessary to keep track of the config data which was used to calculate the breach in the first place? In that case, it doesn't really matter if the breach config data changes - it won't affect historical breaches (unless we deliberately recalculate them), which is the main thing we're trying to preserve? |
@adrianwb really want a single crate that handles all common platforms (WSL is an outlier- I wouldn't mind if it didn't handle that, but should handle Win/Linux/Mac/Android - thanks!) |
I was just trying WSL to avoid having to spin up a "real" Linux distro, so not surprised that it didn't work. I've confirmed today that it does work in a VirtualBox Ubuntu 22 VM, and that it doesn't work when trying to cross-compile for Android (although I've never tried that before so it might be something I've done wrong). I've been unable to find any pre-built crate which does volumes/drives on Android (can't find a way to filter docs.rs results to only show android targets, so no quick solution other than clicking each potential match), so it's looking like I'll just have to use the built-in Rust file handling to look at /mnt/media-rw which seems to be where OTG USB drives are mounted on Android... If anyone knows of something which works for MacOS, that would be nice as well, but that's a bit of an outlier too in terms of our end users ;-^) |
It's more for accountability, if we allow users to edit breach configurations, we would keep historic breaches as is, and if we need to validate our breach configurations we would need to show what it's based on.
Temperature range will be included in the temperature logs that in the breach time range, but as you've mentioned not directly (and maybe not that accurate), and yeah no way of knowing what the actual breach duration threshold was. I think if we don't have the config then we don't need to store it, but if the config is present, it's pretty easy to add that info ?
I think it's only you and Clemens that don't use MacOS, we need to support it for dev purposes (even if customers don't use it). I guess one of MacOSers will have a look at soon enough. Btw I would imagine manually selecting the file is good enough for now (it would also allow upload from browser) I think above would also work for Android |
Yes, that all makes sense, and it's not too hard to store the historical configs - either disallow config edits and always create a new copy when any changes are made (and maybe make the original one inactive so that it isn't used again?), or just store a copy of the config with the breach itself (probably simpler). Either way, this is more of an issue about how omSupply handles the sensor data, rather than how the raw data is generated => part of the omSupply issue rather than this crate.
I was only kidding :-) I'm thinking it would just be a matter of checking
Yes that would work as well.
|
|
I've managed to compile for MacOS (just looking at |
Agree, omSupply issue.
Was also weighing both of those, and I like the later (mainly because we already do something like this and it's in the mSupply schema)
: ), I thought so !
This may help: https://github.com/openmsupply/open-msupply/tree/develop/server/android and then RemoteServer.java, would help further but as far as I know Android is not current priority for this.
I would imagine this is the mvp ? And we can extend after that to use rs-drivelist ? I would assume we would want to get data for sensor where client app is running (or from machine that is running the browser), rather then what is plugged into the server that the client is accessing. Thus maybe we shouldn't' be trying to auto detect (if we do auto detect I think it requires further thinking of actual use cases and go from there i.e. as per above, we would want to access usb of client machine, if it browser then we open up select file dialog, if it's running in client then we can try auto detect with electron) |
|
I realise that above may contradict something we talked about earlier (as per below), I think the detect USB concern should be moved to omSupply client (electron)
|
By downgrading to rust 1.67.0 (as suggested: cross-rs/cross#1222), the libunwind problem goes away and I can actually build for aarch64, which is what I needed to create a test Android app using my Berlinger crate (following the instructions at https://mozilla.github.io/firefox-browser-architecture/experiments/2017-09-21-rust-on-android.html) on my Samsung tablet. The problem now is that my app has no permissions to access USB drives...
I've confirmed that my crate works on Mac, Windows and Linux, but just trying to confirm that the it also works on Android...
Yes, I think you're right that we should do the actual Berlinger file reading on the client side (which can probably handle the auto-detect stuff) and pass a file path to the crate, and then we just pass over the parsed data to the local server => I was then trying to build the openmsupply client, but falling at the first hurdle as yarn can't seem to find |
Cools, I guess if we do it first as upload functionality from browser client (so from server perspective it's an api end point to upload), it would be easily extendable to auto detect, i.e. electron client auto detects usb plugged in and uses the upload end point |
|
I know we can call a Rust crate from Android, but can we also call it directly from the Electron client? Or do we need another public function on this crate which takes the uploaded file contents - something like: pub fn parse_sensor_data(file_contents: &str) -> Option<Sensor> {}; // parse the contents of a Berlinger text filewhich (on the server) could write the contents to a local text file and then use the existing |
|
The electron client is only able to interact with the server through the API, so we would need to add any required functions to (ideally) the graphQL, or the REST layers. The electron client could be on a separate machine to the server as well - though presumably the electron client is on the same machine as the berlinger USB? In which case, we'd have to drop to the node layer in electron to interact with the local file system, read the file, send to the server which saves the text as a file in order to process it |
|
Hi- not sure of the status of this work, but keen to get it working for Technet conference mid-October - thanks @adrianwb |
This (the standalone crate) is already working on Windows. Linux and Mac. In theory it should work on Android as well, but that involves implementing it as an app with the right permissions - rather than spend ages trying to figure all that out from scratch, I decided it was better to get the whole thing integrated into omSupply (#1955), and then (most of) the Android work is already done. There's still a fair bit of work to finish that off, including the UI which is completely new to me => if I don't get selected for jury service on Monday and I get some help with the UI side, then I'd have thought that mid October should be fine to show it as part of omSupply, otherwise we can fallback to the standalone crate. |
|
👋 happy to work on the UI side |
Thanks @mark-prins - I've mostly got #2307 working, but it's not finished yet (see . Hopefully there's enough foundation there to build the rest of #1955 on... |
|
The remaining bit then is to integrate this crate into omSupply, so creating a new branch for that... |
Is your feature request related to a problem? Please describe 👀
Basically we want to port the Berlinger FridgeTag & QTag support from mSupply (e.g. https://github.com/openmsupply/msupply/issues/12331) to omSupply
Describe the solution you'd like 🎁
1. Define a common agreed schema for:
[sensor]table in mSupply)[temperature_breach_config]in mSupply)[temperature_log]in mSupply)[temperature_breach]in mSupply)Note that this is a minimum useful set of fields, but not exhaustive - these tables should be designed for generic use by other sensor types as well. I'm not sure if
2. A new Berlinger-specific crate with the following features:
which provides methods to "query" the structure and return:
Note that for Berlinger sensors, the breach configurations are fixed by the hardware itself and cannot be changed by the user. However, as long as we have raw temperature logs (only available for some Berlinger sensors) we can still apply any arbitrary breach configuration.
3. Add the new schema to omSupply (common for all sensor types):
Note that a given temperature log may be linked to more than one breach e.g. a consecutive breach and a cumulative breach on the same day (especially if we can apply an arbitrary breach configuration!) => rather than attempt to maintain links, it's better to work it out on demand e.g. to get temperature logs for a breach, we query for temperature logs within the breach start/end timestamps which are outside the breach limits; to get breaches for a temperature log, we query for breaches where the temperature log timestamp is within the breach start/end timestamps
4. Add the new crate to omSupply (Berlinger-specific):
5. Design UI for client (common for all sensor types):
Describe alternatives you've considered 💭
Additional context 💌
Moneyworks Jobcode 🧰
The text was updated successfully, but these errors were encountered: