speeduino
speeduino copied to clipboard
noisymime
Pressure Sensor Configuration Updates and ADC Filter fixes
Intended for TPS only configurations. This re-purposes the unused_bit in VVT control which was obsolete and not available for calibration. This memory location is now used for enabling the internal pressure sensor for Baro updates. This setting is mutually exclusive with the existing Use external Baro setting so that it is not possible to enable both.
Baro updates are limited to valid ranges of the MAP sensor and only update at 1kpa per sec to limit any effects of noise or sensor dropout on the baro signal.
As part of any change, I think we probably need to improve some of the wording on these options to indicate exactly what they refer to. 'External Baro sensor' has started to become confusing since these were included on many boards anyway.
Around the method of reading the sensor, why not just do a regular read but with a higher filter value? This should give a more accurate view of the baro pressure I would think.
Yes. I thought the pressure sensor interface should be re-written as: Step 1: Enable and Calibrate Pressure Sensors A, B, C. on Pins XXX Step 2: Assign Pressure sensors A, B, C to signals (MAP, BARO, EMAP) etc. This will require a bit more abstraction in the code about how to read and filter the various signals.
But this PR as it is now is the minimum change required to the existing code to implement a baro feature using the "internal" sensor. Perhaps we can get this one in and then work on a more comprehensive re-write for a future update?
With regards to the filter. I wanted a way to prevent spurious MAP sensor readings from updating baro incorrectly and making major changes to fuelling. Also the filtering should always be very slow since Baro changes slow, 1kpa /sec is the slowest a low pass filter could update the signal anyway. This pr implements both of these requirements with two lines of code. The inital baro value is still set on startup as it was before so a slow filter will not affect cranking.
"As part of any change, I think we probably need to improve some of the wording on these options to indicate exactly what they refer to. 'External Baro sensor' has started to become confusing since these were included on many boards anyway."
- Do you want me to rework the .ini file to remove the assigment of "internal" and "external"? before approving this PR?
I have updated the "Calibrate Pressure Sensors" menu. It now groups together all the pressure sensors including Fuel pressure and Oil pressure on one page. Removed references to "internal" and "External" and generally made things clearer. The functionality is the same as before.
Only one thing is that "Calibrate Pressure Sensors" also exists under the "Accessories" menu which brings up the same dialog. This is where the old "Calibrate Fuel/Oil" menu was. If you want to remove this when this PR is pulled in just search for this line in the .ini and remove it.
subMenu = pressureSensors, "Calibrate Pressure Sensors"
Wow I just realized that the external baro setting has to be set for the onboard baro sensor to work. UA4C board has external baro sensor on A7
In readBaro() you've got the new code for checking the MAP reading (https://github.com/noisymime/speeduino/pull/670/files#diff-cf6893cab52605f0de8b25fc56b15565daa8ea927d1ca9ed99a47ea299d71fcbR493) and updating it, but nothing to prevent this reading happening when the car is running.
Surely you can only be reading the MAP sensor for baro readings when the engine isn't running?
Also, why not just use the existing ADC filter macro?
In
readBaro()you've got the new code for checking the MAP reading (https://github.com/noisymime/speeduino/pull/670/files#diff-cf6893cab52605f0de8b25fc56b15565daa8ea927d1ca9ed99a47ea299d71fcbR493) and updating it, but nothing to prevent this reading happening when the car is running.Surely you can only be reading the MAP sensor for baro readings when the engine isn't running?
Also, why not just use the existing ADC filter macro?
The MAP sensor is not connected to the engine, it's intended for use in a TPS only based setup. The comment for the calibration variable also mentions this.
Not using the ADC macro, because it's not required. Baro should only update very slowly, it's a simple 1 line of code to have it update at a maximum of 1 kpa /sec. To flip this around, why should we spend extra controller resources to update Baro faster?
Also in the same line of code it performs the function of inhibiting a baro update if the MAP sensor value exceeds the range of allowed baro. This prevents a dropout in the MAP sensor from having Baro jump to it's minimum value and affect engine fueling. Remembering that this setup is only supposed to be used with TPS as the fuel load input so that MAP sensor is otherwise not doing anything. This should allow the vehicle to drive well enough to get the owner home if the MAP sensor failed. Continuing the logic from the "read once on startup" function.
Possible development directions - If you want this to be more general, the best way could be to add the flexabiltiy to re-assign the MAP sensor to a different pin, then all the sensors are configurable and users could assign the "baro" sensor to the pin of the internal presure sensor and we don't need this special "Use MAP as Baro" logic. But I need to find a byte of eeprom somewhere for this. - Thoughts?
Ok, as per discussion in #development.
You can enable Baro sensor and assign it to the same Pin as the MAP sensor with no code changes to use the default on-board MAP sensor as a baro sensor. If you are otherwise not using the MAP sensor for anything (i.e. in a TPS only setup) then this is handy. Baro should be filtered pretty heavily. But there is a problem with the inline macro filters in that they do not converge when heavy filtering is applied. I have an easy fix for this which turns the inline macro into a little function with a check for non-convergence. #define ADC_FILTER(input, alpha, prior) (((long)input * (256 - alpha) + ((long)prior * alpha))) >> 8 becomes uint16_t filterADC(uint32_t input, uint32_t alpha, uint32_t prior) { uint16_t result = (uint16_t)prior; if (input != prior) // only need to calculate filter if there is a difference { result = (((input * (256 - alpha)) + (prior * alpha))) >> 8; if (result == (uint16_t)prior) { result = (uint16_t)input; } // Ensures filter convergence to input value with heavy filters. In practice when alpha filters are greater than 128. } // else result = prior = input. and nothing to do.
return result; } 3. Baro when read by an external sensor does not initialize correctly. it starts from a zero value and runs through the filter, this could be a big problem when keying on and cranking immediately with heavier filters. Also a relatively simple fix to initialize the baro value and the range of baro should be limited somewhat in case of a sensor drop-out (as per MAP). 4. Pressure sensor config adds a calibratable pin for MAP and then all sensors are fully adjustable by the user. So whichever physical sensor you want to call "baro" and which one you want to call "MAP" is able to be set by the user. I imagine for many, using a dedicated MAP sensor (likely factory sensor) mounted to the engine manifold and connected to some ADC pin and the on-board sensor for BARO on A0 (most boards) would be a good option.
I did not limit baro to plausible ranges (60 and 105kPa) as I did before. But baro does now have a check for sensor dropout as per MAP.
Requesting again that this PR be pulled in. I merged to the latest master and added a small change to make it more efficient.
To recap this PR does the following:
- Fixed a non-convergence issue with the existing ADC filters - This is a major fix!
- Pressure sensor configuration updates, improving legabiltiy.
- Having No MAP sensor and using the onboard sensor for baro only (with TPS as fuel load) is clearer and neater.
- Ensures that all sensors initalise correctly on startup with no filter for the first loop. Otherwise running resets and quick cranking can cause strange behaviour.
- ~10% Loops/sec performance increase. This is maintained even when ADC filters are turned to maximum.
Testing on the simulator @1500rpm: Baseline Master: 783 Loops/s This PR: 850 Loops/s