Get low-frequency Waveform data from HL7 to Interchange - #172
Get low-frequency Waveform data from HL7 to Interchange#172jeremyestein wants to merge 23 commits into
Conversation
Fully anonymised: - Messages IDs made sequential - All timestamps shifted by same amount - Location changed to another bed - Numerical values changed slightly Also extract the "value type" out of CSV metadata.
create these messages for non-Waveform variables. Not passing them into the collator just yet.
PR checklistDefault guide for a PR (if multiple PRs for the work, only keep one version of it and link to it on the other PRs)
|
message, and fix the tests
thompson318
left a comment
There was a problem hiding this comment.
Looks logical and the workflow makes sense. A few comments / questions.
General comment. I wonder if by keeping "waveform" messages for backward compatibility with what we've already done on the controller we're storing up problems for the future. Would it be better to rename then "waveformHighFreqMessage" to be clear about the two different message types? It would make a lot of work for us for marginal gain so perhaps not.
| record ExpectedWaveformMessage(String variableId, String sourceValue, String mappedUnits, Double numericValue, String stringValue) { | ||
| public void assertIsEqual(WaveformLowFreqMessage actualMessage) { | ||
| assertEquals(variableId, actualMessage.getSourceVariableId()); | ||
| assertEquals(sourceValue, actualMessage.getSourceValue().get()); |
There was a problem hiding this comment.
| assertEquals(sourceValue, actualMessage.getSourceValue().get()); | |
| assertEquals(sourceValue, actualMessage.getSourceValue().get()); | |
| assertEquals(mappedUnits, actualMessage.getMappedUnits().get()); |
Why aren't we checking the mapped units? Would the above suggestion work?
There was a problem hiding this comment.
Well spotted. It's actualMessage.getUnit(), we don't even send the source unit ID. Fixed now.
There was a problem hiding this comment.
Lovely flow chart, makes it clear what's supposed to happen.
| logger.trace("HL7 message generated {} Waveform messages ({} collatable, {} not), sending for collation", | ||
| msgs.size(), waveformMessages.size(), lfMessages.size()); | ||
| for (var m: lfMessages) { | ||
| waveformOperations.sendMessage(m); |
There was a problem hiding this comment.
What's the rationale for not sending low frequency messages to a collator?
There was a problem hiding this comment.
Low frequency messages don't have a fixed sampling frequency in the same way that HF ones do, so I decided we should store them as one row (CSV row or emap-star row) per data point, as opposed to arrays that we already use for the HF data.
In practice, they seem to be sent 3 times a second (haven't checked intervals for exactness), but my take was that this is probably configurable and not an inherent property of the source data.
But yes, you could treat these exactly like an HF data source, and reduce them down to a start time and an array, from which you have to work out the time of any particular point by knowing its position in the array and the sampling frequency (start_time + idx / samp_freq).
And maybe 3/sec will create an annoyingly long CSV/parquet file, and we'll wish we used arrays.
Another option, if we're not sure the intervals are spot on, is to squash them into periods with a start and end date where the value is the same (good for things like ventilation mode that rarely change, but maybe bad for other things?), as we talked about in one of the meetings this week. But this would be extra work.
Maybe we should talk about this more?
It wouldn't be that much work to rename. And I'm currently working on the waveform controller code anyway (which, as it happens, doesn't check the message type anyway). So it would be possible to change it if you think it's better. |
If it's not much work then I suggest renaming it. I'm trying to think from the perspective of someone in a couple of years time who get's asked to implement a new type of waveform message, and may struggle to understand the inheritance structure as is. |
Start at
Hl7ParseAndQueue.javaif you want to see the main part of the change first.Process low frequency waveform data, eg. settings, from HL7 to Emap interchange messages. These are scalar values reported at a lower frequency (typically 3Hz) and can measure anything, but are not actually waveforms themselves.
WaveformMessageinto abstract base classWaveformBaseMessageand add another subclassWaveformLowFreqMessagefor low frequency messages. Kept original name for compatibility with downstream waveform controller.