Questions about the implementation

It's all about the code!
Post Reply
ssmith
Posts: 92
Joined: Sun Oct 17, 2021 10:21 pm

Questions about the implementation

Post by ssmith »

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?
mck1117
running engine in first post
running engine in first post
Posts: 1493
Joined: Mon Jan 30, 2017 2:05 am
Location: Seattle-ish

Re: Questions about the implementation

Post by mck1117 »

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.
User avatar
AndreyB
Site Admin
Posts: 14292
Joined: Wed Aug 28, 2013 1:28 am
Location: Jersey City
Github Username: rusefillc
Slack: Andrey B

Re: Questions about the implementation

Post by AndreyB »

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
mck1117
running engine in first post
running engine in first post
Posts: 1493
Joined: Mon Jan 30, 2017 2:05 am
Location: Seattle-ish

Re: Questions about the implementation

Post by mck1117 »

ssmith wrote:
Sat Oct 30, 2021 12:55 am
I've been reading over some of the code and have come up with some questions. Sorry if this is newbie stuff
Oh, and this isn't newbie stuff. Asking this kind of question indicates that you have a very deep level of understanding about what's going on here ;)
ssmith
Posts: 92
Joined: Sun Oct 17, 2021 10:21 pm

Re: Questions about the implementation

Post by ssmith »

mck1117 wrote:
Sat Oct 30, 2021 1:24 am
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.
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.

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.
mck1117 wrote:
Sat Oct 30, 2021 1:24 am
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.
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/Tkinter ;-)

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.
mck1117 wrote:
Sat Oct 30, 2021 1:24 am
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.
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....
User avatar
AndreyB
Site Admin
Posts: 14292
Joined: Wed Aug 28, 2013 1:28 am
Location: Jersey City
Github Username: rusefillc
Slack: Andrey B

Re: Questions about the implementation

Post by AndreyB »

ssmith wrote:
Sat Oct 30, 2021 3:55 am
Is there much desire to move away from TunerStudio, or that deemed too much effort?
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
mk e
Posts: 486
Joined: Tue Dec 06, 2016 7:32 pm

Re: Questions about the implementation

Post by mk e »

mck1117 wrote:
Sat Oct 30, 2021 1:24 am
3. Yes, this is a known thing. As you guessed it's a little fiddly to make the transition seamless for users.
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 :oops:

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
MAP smoothing 2017_07_18Capture.JPG
MAP smoothing 2017_07_18Capture.JPG (65.35 KiB) Viewed 7946 times
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
MAP smoothing 2017_07_18 table.JPG (66.98 KiB) Viewed 7946 times
ssmith
Posts: 92
Joined: Sun Oct 17, 2021 10:21 pm

Re: Questions about the implementation

Post by ssmith »

mck1117 wrote:
Sat Oct 30, 2021 1:24 am
3. Yes, this is a known thing. As you guessed it's a little fiddly to make the transition seamless for users.
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.
mck1117
running engine in first post
running engine in first post
Posts: 1493
Joined: Mon Jan 30, 2017 2:05 am
Location: Seattle-ish

Re: Questions about the implementation

Post by mck1117 »

ssmith wrote:
Mon Nov 01, 2021 6:39 pm
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!

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.
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 pm
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.
We should do this, can do this without any change to TS.
ssmith
Posts: 92
Joined: Sun Oct 17, 2021 10:21 pm

Re: Questions about the implementation

Post by ssmith »

mck1117 wrote:
Mon Nov 01, 2021 6:45 pm
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.
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?

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?)
mck1117 wrote:
Mon Nov 01, 2021 6:45 pm
ssmith wrote:
Mon Nov 01, 2021 6:39 pm
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.
We should do this, can do this without any change to TS.
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
running engine in first post
running engine in first post
Posts: 1493
Joined: Mon Jan 30, 2017 2:05 am
Location: Seattle-ish

Re: Questions about the implementation

Post by mck1117 »

ssmith wrote:
Mon Nov 01, 2021 7:42 pm
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.
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.
ssmith
Posts: 92
Joined: Sun Oct 17, 2021 10:21 pm

Re: Questions about the implementation

Post by ssmith »

mck1117 wrote:
Tue Nov 02, 2021 12:50 am
ssmith wrote:
Mon Nov 01, 2021 7:42 pm
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.
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.
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.
User avatar
Dron_Gus
contributor
contributor
Posts: 450
Joined: Wed Nov 13, 2013 1:11 pm
Location: S-Pb
Github Username: dron0gus

Re: Questions about the implementation

Post by Dron_Gus »

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.
mck1117
running engine in first post
running engine in first post
Posts: 1493
Joined: Mon Jan 30, 2017 2:05 am
Location: Seattle-ish

Re: Questions about the implementation

Post by mck1117 »

mk e wrote:
Sat Oct 30, 2021 1:01 pm
the orange saw tooth is crank angle so you can see phasing
Say, that's a good idea for us to add. We've prodded the tunerstudio folks enough so that we can now update/log at a full 1khz, so engine phase is a fun thing to log.
mk e
Posts: 486
Joined: Tue Dec 06, 2016 7:32 pm

Re: Questions about the implementation

Post by mk e »

mck1117 wrote:
Tue Nov 02, 2021 11:02 pm

Say, that's a good idea for us to add. We've prodded the tunerstudio folks enough so that we can now update/log at a full 1khz, so engine phase is a fun thing to log.
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.
mck1117
running engine in first post
running engine in first post
Posts: 1493
Joined: Mon Jan 30, 2017 2:05 am
Location: Seattle-ish

Re: Questions about the implementation

Post by mck1117 »

Dron_Gus wrote:
Tue Nov 02, 2021 9:26 am
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.
what value do we think that would add?
mk e
Posts: 486
Joined: Tue Dec 06, 2016 7:32 pm

Re: Questions about the implementation

Post by mk e »

mck1117 wrote:
Wed Nov 03, 2021 9:18 pm
Dron_Gus wrote:
Tue Nov 02, 2021 9:26 am
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.
what value do we think that would add?
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.
Attachments
Table.JPG
Table.JPG (77.82 KiB) Viewed 7465 times
User avatar
Dron_Gus
contributor
contributor
Posts: 450
Joined: Wed Nov 13, 2013 1:11 pm
Location: S-Pb
Github Username: dron0gus

Re: Questions about the implementation

Post by Dron_Gus »

mck1117 wrote:
Wed Nov 03, 2021 9:18 pm
Dron_Gus wrote:
Tue Nov 02, 2021 9:26 am
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.
what value do we think that would add?
Switchable tables. Saved RAM.
mk e
Posts: 486
Joined: Tue Dec 06, 2016 7:32 pm

Re: Questions about the implementation

Post by mk e »

Dron_Gus wrote:
Thu Nov 04, 2021 9:32 pm

Switchable tables. Saved RAM.
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.
Post Reply