Questions about the implementation
Questions about the implementation
I've been reading over some of the code and have come up with some questions. Sorry if this is newbie stuff, but here goes:
1. I see some rather complicated logic around MAP averaging. I assume the idea is to smooth out the pulse that happens as the valves open and close. Any reason not to just use a simple RC circuit to low pass at 100Hz, reducing max sample rate from 10kHz to 1kHz-ish? Given most wiring harnesses don't use a shielded or twisted cable for MAP (unlike knock sensors), it seems overkill to sample so high. i.e. is it really necessary to track cylinder to cylinder differences in MAP at 9000RPM, or is having the average for the whole crank rotation sufficient? I could see needing additional filtering at very low RPM though, so perhaps the complexity remains, but at a lower frequency.
2. A lot of tables seem to have hard coded sizes and axis. I assume this is a limitation of TunerStudio? Some aftermarket EMS let you change the axis to be anything you want, which is gives the user a lot of flexibility, esp when the output of one table can used as an input to another table. Is there a goal to move in that direction, or is the idea that if users want to change anything they need to edit the source code? I guess one possibility would be to implement a rudimentary table constructor using the console, and then have that spit out a custom TS ini file.
3. Some tables seemed overprovisioned in accuracy; the ignition table uses a float instead of an int16_t in 0.01 degrees, the VE table is a float instead of 0.01%, etc. Is this easy to change or would it break compatibility with existing tunes (does TunerStudio convert appropriately between configurations?) I did see a pending pull request that addresses some of the load bins, but not the tables themselves.
4. I guess a hardware question - is there a reason the pins are protected with op-amps instead of relying on input impedance (some sort of RC circuit, maybe voltage divider since the MCU is a 3.3V part) and built in pin diodes?
1. I see some rather complicated logic around MAP averaging. I assume the idea is to smooth out the pulse that happens as the valves open and close. Any reason not to just use a simple RC circuit to low pass at 100Hz, reducing max sample rate from 10kHz to 1kHz-ish? Given most wiring harnesses don't use a shielded or twisted cable for MAP (unlike knock sensors), it seems overkill to sample so high. i.e. is it really necessary to track cylinder to cylinder differences in MAP at 9000RPM, or is having the average for the whole crank rotation sufficient? I could see needing additional filtering at very low RPM though, so perhaps the complexity remains, but at a lower frequency.
2. A lot of tables seem to have hard coded sizes and axis. I assume this is a limitation of TunerStudio? Some aftermarket EMS let you change the axis to be anything you want, which is gives the user a lot of flexibility, esp when the output of one table can used as an input to another table. Is there a goal to move in that direction, or is the idea that if users want to change anything they need to edit the source code? I guess one possibility would be to implement a rudimentary table constructor using the console, and then have that spit out a custom TS ini file.
3. Some tables seemed overprovisioned in accuracy; the ignition table uses a float instead of an int16_t in 0.01 degrees, the VE table is a float instead of 0.01%, etc. Is this easy to change or would it break compatibility with existing tunes (does TunerStudio convert appropriately between configurations?) I did see a pending pull request that addresses some of the load bins, but not the tables themselves.
4. I guess a hardware question - is there a reason the pins are protected with op-amps instead of relying on input impedance (some sort of RC circuit, maybe voltage divider since the MCU is a 3.3V part) and built in pin diodes?
-
- running engine in first post
- Posts: 1494
- Joined: Mon Jan 30, 2017 2:05 am
- Location: Seattle-ish
Re: Questions about the implementation
1. MAP averaging is particularly useful for engines with ITBs or tiny plenums, where the MAP signal may only be valid for a short duration (ie, intake valve is open). Phase aligned sampling also can help avoid any aliasing since the MAP ripple is of course variable frequency.
2. It's a possibility, but we have the resources that we can sort of just make them all big, and not worry about it. It would be exceedingly complicated to get TS to work with a dynamic table layout.
3. Yes, this is a known thing. As you guessed it's a little fiddly to make the transition seamless for users.
4. Our analog inputs are 0-5v, but the MCU is 0-3.3v. If you want to use a plain divider, it means you need to have a very low input impedance external to the ECU. That makes it exceptionally hard to protect against transients - a good recipe for a big zap directly in to your relatively fragile ADC. So instead we use a series resistor to limit current, clamping diodes, a cap for an anti aliasing filter (along with the series R), the opamp to make a low impedance copy, then a resistor divider to drop 5v to 3.3.
2. It's a possibility, but we have the resources that we can sort of just make them all big, and not worry about it. It would be exceedingly complicated to get TS to work with a dynamic table layout.
3. Yes, this is a known thing. As you guessed it's a little fiddly to make the transition seamless for users.
4. Our analog inputs are 0-5v, but the MCU is 0-3.3v. If you want to use a plain divider, it means you need to have a very low input impedance external to the ECU. That makes it exceptionally hard to protect against transients - a good recipe for a big zap directly in to your relatively fragile ADC. So instead we use a series resistor to limit current, clamping diodes, a cap for an anti aliasing filter (along with the series R), the opamp to make a low impedance copy, then a resistor divider to drop 5v to 3.3.
- AndreyB
- Site Admin
- Posts: 14345
- Joined: Wed Aug 28, 2013 1:28 am
- Location: Jersey City
- Github Username: rusefillc
- Slack: Andrey B
Re: Questions about the implementation
Yes.
Very limited telepathic abilities - please post logs & tunes where appropriate - http://rusefi.com/s/questions
Always looking for C/C++/Java/PHP developers! Please help us see https://rusefi.com/s/howtocontribute
Always looking for C/C++/Java/PHP developers! Please help us see https://rusefi.com/s/howtocontribute
Re: Questions about the implementation
That would seem to require a fast responding MAP and low noise wiring. I'm just surprised that works well enough with ITB to use MAP instead of TPS, though I guess the BMW E46 M3 CSL used a MAP sensor with ITB. I'm not sure about the details of the implementation though. With an inline 6 I'm not sure there's ever a time when the intake valves are all closed In the past I've tuned BMW S54 motors using TPS instead of MAP precisely because of the ITB/MAP issue.mck1117 wrote: ↑Sat Oct 30, 2021 1:24 am1. MAP averaging is particularly useful for engines with ITBs or tiny plenums, where the MAP signal may only be valid for a short duration (ie, intake valve is open). Phase aligned sampling also can help avoid any aliasing since the MAP ripple is of course variable frequency.
It was actually thanks to you guys that I fixed a bug in my boost controller for my turbo 4 cyl. I have MAP sensors both before and after the throttle body; the Link ECU uses the one in the manifold while my boost controller uses the one in the charge pipe. I didn't understand why I had a ripple at 6000+-200 RPM that looked very much like resonance. Finally figured out it was aliasing due to sampling at 100Hz; when I bumped it to 1000Hz with a strong low pass filter, the problem went away. That's not with ITB though, and it's before the ITB anyways, but it showed me the need to average even at full throttle.
Is there much desire to move away from TunerStudio, or that deemed too much effort? I see that there's EMStudio as a potential starting point but I have basically 0 experience with either product, and alas I'm not really a UI guy. Maybe enough to modify EMStudio but not to spin one up from scratch, unless you like Python/Tkintermck1117 wrote: ↑Sat Oct 30, 2021 1:24 am2. It's a possibility, but we have the resources that we can sort of just make them all big, and not worry about it. It would be exceedingly complicated to get TS to work with a dynamic table layout.
3. Yes, this is a known thing. As you guessed it's a little fiddly to make the transition seamless for users.
I have dreams of a self describing tuning binary file format that contains both the configuration and the data; on reception the ECU would simply validate the configuration is as expected before accepting the data (otherwise outright refuse it). The tuning software would be responsible for downloading the latest config from the ECU before burning the replacement to make sure the data was converted correctly. Users wouldn't start with ini files as projects, but instead start with "empty" basemaps to avoid having to match ini files to maps.
I thought the capacitor in the RC low pass filter would provide a low enough impedance path for the ADC. At one point I convinced myself if the pin and ADC internal buffer capacitance multiplied by the sampling rate was less than the C in the RC circuit by a factor of 10-ish, then it wouldn't be a problem, but I haven't done the math in a while. But if your sample rate is 10kHz, then that's not likely to happen....mck1117 wrote: ↑Sat Oct 30, 2021 1:24 am4. Our analog inputs are 0-5v, but the MCU is 0-3.3v. If you want to use a plain divider, it means you need to have a very low input impedance external to the ECU. That makes it exceptionally hard to protect against transients - a good recipe for a big zap directly in to your relatively fragile ADC. So instead we use a series resistor to limit current, clamping diodes, a cap for an anti aliasing filter (along with the series R), the opamp to make a low impedance copy, then a resistor divider to drop 5v to 3.3.
- AndreyB
- Site Admin
- Posts: 14345
- Joined: Wed Aug 28, 2013 1:28 am
- Location: Jersey City
- Github Username: rusefillc
- Slack: Andrey B
Re: Questions about the implementation
Priorities. TunerStudio is pretty good in a lot of ways. I would much rather make progress on a huge list of other things.
Very limited telepathic abilities - please post logs & tunes where appropriate - http://rusefi.com/s/questions
Always looking for C/C++/Java/PHP developers! Please help us see https://rusefi.com/s/howtocontribute
Always looking for C/C++/Java/PHP developers! Please help us see https://rusefi.com/s/howtocontribute
Re: Questions about the implementation
I didn't look to see if you are using it but TS added a 32 bit float, F32 as a variable type for us on the o5e project, didn't look to see if you're using it, or maybe they have since dropped it? It used to be there though but did create small problems with comparing stored tune to existing tune, TS would often says something changed when it hadn't.
Do you guys have any data on tracking ITBs by reading the MAP fast? My ECU reads at 2k and I never got data I loved is why I ask, just wondering if others had better luck.
Here is some MAP data from my 12cyl running when I was playing with different ways to process the MAP signal still not fully realizing the raw signal it looked the way it looked because the engine was broken and the spikes up are dead cylinders but I thought I just had a data processing issue
the orange saw tooth is crank angle so you can see phasing, the Red the raw MAP data(but remember that is raw from the multiMAP so lowest of 12 MAP sensors), Blue a simple running average that makes it smooth but very laggy, the cyan is what I currently use
Its just a running average but I added a table for the coefficient to use and it seems to work quite well but I did want to play a bit more once I'm running again.
MAP= X*MAP+(1-X)*MAPREADING
- Attachments
-
- MAP smoothing 2017_07_18 table.JPG (66.98 KiB) Viewed 8293 times
Re: Questions about the implementation
Well I started down the path of writing a rom converter (actually finished it, ~120 lines of python), but then it dawned on me you might already have something like that? I notice rusefi online has XML versions of tuning maps, which means maybe there's a way to turn that back into a ROM? If not, then maybe this would be useful for reorganizing the flash memory and changing the types/multipliers. It doesn't currently handle array resizing though (i.e. 8x8->12x10).
It seems to me that all the arrays should be at most u16/s16 types with proper multipliers. It gives plenty of resolution (.01%, .01 degrees, .001V, .1kpa unless GDI, etc) but still being reasonably compact (a 20x16 table would be 2*(20+16+20*16) 712 bytes. Converting all the F32 tables to S16/U16 would save 5k given the existing table sizes.
Another wastage is since you can't disable tables (TunerStudio doesn't seem to support dynamically finding where tables are?), you need to allocate memory for all the tables anyone might possibly want, even if they don't need it. For example, I have no need for the iacPidMultTable (my engine doesn't have an iac), but I would need GDI/HPFP tables (which most people don't want). I don't need the tcuSolenoidTable, etc.
Lua/FSIO is also affecting memory usage (>3k), again there whether it's being used or not.
IMO TunerStudio is getting in the way of both efficient memory layout (maybe you don't care because you'll just bump the minimum required MCU and leave existing users with no upgrade path?) and in the way of usability without recompiling the firmware (for traction control I want a 2d table based on RPM! No, 3d table based on RPM and TPS! No, I want an extra adder table on top of that based on lateral G forces! Oh and maybe gear! - each iteration would require a recompile instead of simply clicking a few buttons in a GUI).
Also I think changing the format once (while annoying) would make upgrade paths easier. By predetermining that there is a fixed scalar/bits section and a separate array/"string" section, you can make it easier to add to the scalar/bits section without chopping up the array section, making it harder to reclaim that memory if you resize/refactor the arrays. For example, the rom layout could be:
u16 scalar_len;
u8 scalar_zone[scalar_len];
u16 num_tables;
u16 table_offset[num_tables];
// per table:
u16 x_axis_variable : 11;
u16 x_axis_len : 5; // 0=size 1
u16 y_axis_variable : 11; // variable also codes multiplier used, for example MAP would be 0.1kpa but HPFP would be 1kpa or even 10kpa
u16 y_axis_len : 5; // 0=size 1
u16 x_axis_bins[x_axis_len];
u16 y_axis_bins[y_axis_len];
u16 data[x_axis_len*y_axis_len]; // units and multiplier are coded by the user of the table
Then if you install a new firmware that has a scalar_len of 831 instead of the 829 that's stored in the current tune, it can know to pad those 2 bytes with 0 when it copies the tune to RAM. The next time the tuning software flashes a new configuration, it can update the layout. New firmware only increases the size of the tune for the scalars (minimal) and the tables actually used. Users with newer hardware can use more features than older hardware, but users with older hardware still get bug fixes and other improvements that come with new software versions.
I guess I really believe that either a TunerStudio replacement or improvements to the TunerStudio configuration would go a long ways toward improving the flexibility of RusEFI.
-
- running engine in first post
- Posts: 1494
- Joined: Mon Jan 30, 2017 2:05 am
- Location: Seattle-ish
Re: Questions about the implementation
Correct, TS is absolutely a limitation of how we can shape the config. The question is not how to design a memory layout that could do something different/fun/interesting, but rewriting an entire GUI tuning app to support that, along with the logging and pretty UI that TS already has implemented.ssmith wrote: ↑Mon Nov 01, 2021 6:39 pmIMO TunerStudio is getting in the way of both efficient memory layout (maybe you don't care because you'll just bump the minimum required MCU and leave existing users with no upgrade path?) and in the way of usability without recompiling the firmware (for traction control I want a 2d table based on RPM! No, 3d table based on RPM and TPS!
I guess I really believe that either a TunerStudio replacement or improvements to the TunerStudio configuration would go a long ways toward improving the flexibility of RusEFI.
We should do this, can do this without any change to TS.ssmith wrote: ↑Mon Nov 01, 2021 6:39 pmIt seems to me that all the arrays should be at most u16/s16 types with proper multipliers. It gives plenty of resolution (.01%, .01 degrees, .001V, .1kpa unless GDI, etc) but still being reasonably compact (a 20x16 table would be 2*(20+16+20*16) 712 bytes. Converting all the F32 tables to S16/U16 would save 5k given the existing table sizes.
Re: Questions about the implementation
You lost me at "pretty UI" But this going back to an earlier proposal - what about extending RusEFI console or having a separate app that allows you to do some table configuration (yes I want these tables, this many buckets, these source variables, combine the tables in this way), and then have it spit out an INI file for TunerStudio to use?mck1117 wrote: ↑Mon Nov 01, 2021 6:45 pmCorrect, TS is absolutely a limitation of how we can shape the config. The question is not how to design a memory layout that could do something different/fun/interesting, but rewriting an entire GUI tuning app to support that, along with the logging and pretty UI that TS already has implemented.
Maybe I'll chip away at this in the background. I actually disagree that it would be a lot of work to get a tuning and logging app up and running, but I may as well put my time where my mouth is. At the very least it'll give me a way to run the tuning software against the simulator on Linux (I tried socat to create a virtual com port, but it seems TunerStudio senses that it isn't a real com port - it might be because it fiddles with the baud rate between 115200 and 9600 perhaps looking for failures at 9600?)
Silly me, I didn't realize an MSQ file is just XML. So it seems changing the table layout is possible using just TunerStudio. You'd ideally want some way to notice the given tune is for an old layout and refuse to run it, so users are forced to go back through TunerStudio to realign the tune to the layout.mck1117 wrote: ↑Mon Nov 01, 2021 6:45 pmWe should do this, can do this without any change to TS.ssmith wrote: ↑Mon Nov 01, 2021 6:39 pmIt seems to me that all the arrays should be at most u16/s16 types with proper multipliers. It gives plenty of resolution (.01%, .01 degrees, .001V, .1kpa unless GDI, etc) but still being reasonably compact (a 20x16 table would be 2*(20+16+20*16) 712 bytes. Converting all the F32 tables to S16/U16 would save 5k given the existing table sizes.
-
- running engine in first post
- Posts: 1494
- Joined: Mon Jan 30, 2017 2:05 am
- Location: Seattle-ish
Re: Questions about the implementation
My day job is in application software. Trust me, the gulf between "up and running" and "usable" is far, far wider than you think it is.
Re: Questions about the implementation
Yeah the 90/10 rule definitely applies to GUI development. Refinement is hard. Though given RusEFI is 8+ years old, and you only last year fixed some major performance issues in the timer code, it seems the same applies to the firmware.
- Dron_Gus
- contributor
- Posts: 456
- Joined: Wed Nov 13, 2013 1:11 pm
- Location: S-Pb
- Github Username: dron0gus
Re: Questions about the implementation
Few fought:
1. keep settings, tables as big as you want. On storage. Preload only used tables in ram. Can also solve request having several switchable tables. Will need some kind on software MMU. Dynamic allocation. Can cause out of ram issue during execution.
2. generate ini files on MCU side. This will allow user to resize tables without recompilation.
1. keep settings, tables as big as you want. On storage. Preload only used tables in ram. Can also solve request having several switchable tables. Will need some kind on software MMU. Dynamic allocation. Can cause out of ram issue during execution.
2. generate ini files on MCU side. This will allow user to resize tables without recompilation.
Re: Questions about the implementation
Even a blind squirrel finds a nut ever now and then, and even I can think of something useful now and again
With the ECU I use, I can log to the PC at 1khz, but that only really works for the EAL (enginelab abstraction layer) items which are low level that is read-only to me. So I can look at crank position or say AN8 voltage at 1khz, but MAP that uses AN8 is was on a 2ms sleep thread for this test (normally 7ms) so 500hz at best, 200-300hz in reality and you see that in the steps in the log that are longer on MAP than crank. Logging AN8 at 1khz can tell me if there is noise on the signal, and I can set a low level filter (again a running average) but the value used for MAP is whatever the voltage was when the loop with the MAP stuff ran and I'm doing my filtering in that loop. In theory I can set a thread to sleep 1ms, so 1khz but it is a sleep time not a run time so actual results vary and are never 1khz. Logging to the PC though logs every channel that is displayed anywhere on the layout so files get big fast at 1khz so I only use that when I'm looking for something specific as I was here trying to see MAP cylinder by cylinder This is all when logging to the PC...I think the USB logging is 20hz max I think there may be a way to change the base rate higher but I've not seen it, for sure the rate can be slowed for any channel, and only channels I preselect are logged, but I haven't played with it at all, just glanced at it for the if my car ever runs day.
-
- running engine in first post
- Posts: 1494
- Joined: Mon Jan 30, 2017 2:05 am
- Location: Seattle-ish
Re: Questions about the implementation
what value do we think that would add?Dron_Gus wrote: ↑Tue Nov 02, 2021 9:26 amFew fought:
1. keep settings, tables as big as you want. On storage. Preload only used tables in ram. Can also solve request having several switchable tables. Will need some kind on software MMU. Dynamic allocation. Can cause out of ram issue during execution.
2. generate ini files on MCU side. This will allow user to resize tables without recompilation.
Re: Questions about the implementation
Not sure this is what he was thinking but in the setup I use, when I create a table I set the max size but can hide row/columns so something like the spark table can be set to say 32x32max, but displayed as 16x16 by default...then the user can right click and select "add row" or "remove column" and see/use a larger or smaller table on the fly. This makes initial rough tune go faster with less planning as you know you can insert a row/column where ever you need to later with no fuss. TS would need to support it though so probably not possible in the short term...but it is a nice to have.mck1117 wrote: ↑Wed Nov 03, 2021 9:18 pmwhat value do we think that would add?Dron_Gus wrote: ↑Tue Nov 02, 2021 9:26 amFew fought:
1. keep settings, tables as big as you want. On storage. Preload only used tables in ram. Can also solve request having several switchable tables. Will need some kind on software MMU. Dynamic allocation. Can cause out of ram issue during execution.
2. generate ini files on MCU side. This will allow user to resize tables without recompilation.
- Attachments
-
- Table.JPG (77.82 KiB) Viewed 7812 times
- Dron_Gus
- contributor
- Posts: 456
- Joined: Wed Nov 13, 2013 1:11 pm
- Location: S-Pb
- Github Username: dron0gus
Re: Questions about the implementation
Switchable tables. Saved RAM.mck1117 wrote: ↑Wed Nov 03, 2021 9:18 pmwhat value do we think that would add?Dron_Gus wrote: ↑Tue Nov 02, 2021 9:26 amFew fought:
1. keep settings, tables as big as you want. On storage. Preload only used tables in ram. Can also solve request having several switchable tables. Will need some kind on software MMU. Dynamic allocation. Can cause out of ram issue during execution.
2. generate ini files on MCU side. This will allow user to resize tables without recompilation.
Re: Questions about the implementation
I'm pretty sure the enginelab table resizing doesn't save ram, its just a user friendliness feature....I might be wrong though. Motec has a similar feature, I have no idea how theirs actually works, its old, they have had the feature since back when RAM really mattered so it could be what you're describing.