Closed ethanjli closed 3 years ago
@KorlaMarch Would it be easy for you to report what are the values of the dp_scale
scale factors currently provided by each of our SDP sensors, so that whoever takes on this issue can hard-code them into the configurations for the sensors?
@KorlaMarch Would it be easy for you to report what are the values of the
dp_scale
scale factors currently provided by each of our SDP sensors, so that whoever takes on this issue can hard-code them into the configurations for the sensors?
Yes, but I wouldn't hardcoded the value. Would it be possible to do full 6-bytes read at the startup or occasionally (for example, once every second) and use those values instead? we might still want to monitor the temperature, just not all the time.
Interesting. In that case, I guess we would need to modify SDPSensor::readSample(...)
and SDPSensor::parseReading(...)
so that we can specify whether to do a 2-byte read (just to get differential pressure) or a full 6-byte read (to also get temperature and the differential pressure scaling factor). I can think of two approaches for this in which we don't silently cache the scaling factor inside SDPSensor
:
I2CDeviceStatus SDPSensor::readSample(SDPSample &sample)
method which does a full 6-byte read, and a I2CDeviceStatus readSample(int16_t differentialPressureScale, float &differentialPressure);
method which just does a 2-byte read and uses the provided scaling factor to compute the differential pressure.I2CDeviceStatus SDPSensor::readFullSample(SDPSample &sample)
method which does a full 6-byte read, and a I2CDeviceStatus readPressureSample(int16_t differentialPressureScale, float &differentialPressure);
method which just does a 2-byte read and uses the provided scaling factor to compute the differential pressure.The method interfaces would be kind of different, while the implementations of those methods would be the same. Any thoughts? I usually prefer method overloading, but in this case I can also see a compelling reason to use differently-named methods: we want to be very explicit that the two numbers provided for the 2-byte read have to be in a specific order.
Started analysing methods which are present in SDP.cpp to reduce I2C traffic from SDP sensor. Required clarification At start up we have to read full 6 bytes of data and from next time will request only first 2 bytes of data or how it is? Please clarify.
We will need to provide two different methods with different behaviors: one method requests and processes the full 6 bytes of data; and the other method requests and processes only the first 2 bytes of data. We have not yet decided when the first method will be called (it might be not only during startup, but might also be at regular intervals, e.g. once per minute or once per ten minutes); if we just provide both of these two methods and allow them to be called independently at different times, then we can delay our decision about when we will request+process 6 bytes of data vs. when we will request+process 2 bytes of data.
Defined a method I2CDeviceStatus SDPSensor::readPressureSample (int16_t differentialPressureScale, float &differentialPressure); to read first 2-bytes of data and calculating pressuresample from data[0] and data[1].
differentialPressure = pressuresample/(differentialPressureScale);
Before reading first 2 bytes of data we have to read 6 bytes of data to get the scaling factor (differentialPressureScale) or we can take scaling factor as a fixed value?
For that readPressureSample
method which only reads the first 2 bytes of data, you can assume that the caller of the method already knows the scaling factor (e.g. because the caller had previously called the 6-byte version of the method and saved the scaling factor outputted by that method) and has provided it as a parameter to the 2-byte readPressureSample
method.
In
firmware/ventilator-controller-stm32/Core/Src/Pufferfish/Driver/I2C/SDP.cpp
, currentlySDPSensor::readSample(...)
is requesting 6 bytes of data from the SDP differential pressure sensor. However, we only really need the first two bytes, which contain the differential pressure measurement; the next two bytes contain a temperature measurement (which we always ignore) calledtemp_raw
inSDPSensor::parseReading(...)
, and the final two bytes contain a differential pressure scaling factor(which we can just hard-code through the sensor configuration passed to thecalledSDPSensor::SDPSensor(...)
constructor, once we determine the value of that scale factor)dp_scale
inSDPSensor::parseReading(...)
.Instead of requesting and receiving 6 bytes of data with every single sensor reading, we would like to only request the first 2 bytes of data which what we actually care about. This will involve modifying the implementations of
SDPSensor::readSample(...)
andSDPSensor::parseReading(...)
(most likely changing the value ofDATA_LEN
and any code usingdata
), ensuring that the modification conforms to the I2C protocol specification for the SDP sensors (refer to datasheets below), and, ideally, testing the resulting implementation to ensure that it works. If you do not have easy access to the sensors for testing, just open a pull request with the implementation and ask @KorlaMarch to test your implementation.As mentioned above, the differential pressure scaling factor should be hard-coded into a sensor configuration struct to be passed to the constructor forSee below discussion for what we should do with the differential pressure scaling factor.SDPSensor
. Please refer to theABPConfig
struct, the constructor of theHoneywellABP
class, and thestatic constexpr ABPConfig ...
members at the top of theHoneywellABP
class, all in thefirmware/ventilator-controller-stm32/Core/Src/Pufferfish/Driver/I2C/HoneywellABP.cpp
file, for an example of how the sensor configuration struct forSDPSensor
should be set up. You will need to create anSDPConfig
struct which has adifferentialPressureScale
member and ai2cAddr
member.This solution needs to work with both our SDP8xx sensors and our SDP3x sensors.
Additional context