Notices
MEGAsquirt A place to collectively sort out this megasquirt gizmo

DIYPNP install: inital tuning

Thread Tools
 
Search this Thread
 
Old May 30, 2011 | 07:56 PM
  #201  
Greg G's Avatar
Thread Starter
Junior Member
 
Joined: Jun 2007
Posts: 411
Total Cats: 0
Default

From MSExtra:
If you thought that's what I was saying, you misunderstood.

There should never be a reason to enter negative numbers for the D term.

About the only thing I'd do in code is amplify its effect so you don't have to use such a large number for it.

Ken
Old May 30, 2011 | 08:22 PM
  #202  
Greg G's Avatar
Thread Starter
Junior Member
 
Joined: Jun 2007
Posts: 411
Total Cats: 0
Default

Hey Jason. Would getting the D response in 1 tick faster significantly shift the idle duty sinusoid to the left?
Old May 31, 2011 | 06:01 PM
  #203  
JasonC SBB's Avatar
Elite Member
 
Joined: Jul 2005
Posts: 6,420
Total Cats: 84
Default

Doesn't appear to be anywhere as significant an issue as the inverted sign.
Old Jun 13, 2011 | 02:18 PM
  #204  
JasonC SBB's Avatar
Elite Member
 
Joined: Jul 2005
Posts: 6,420
Total Cats: 84
Default

Here's a code snippet from ms2_extra_idle.c

tmp1 = IACmotor_pos - (((long)(Kp + Ki - Kd) * ...

Why is Kd subtracted from the others and not added??
This appears to be the inverted sign problem.

How hard is it to get someone to change the sign, recompile, and for Greg to flash the new program in?
Old Jun 13, 2011 | 02:25 PM
  #205  
Braineack's Avatar
Boost Czar
iTrader: (62)
 
Joined: May 2005
Posts: 80,552
Total Cats: 4,368
From: Chantilly, VA
Default

ask reverant.
Old Jun 13, 2011 | 07:15 PM
  #206  
Greg G's Avatar
Thread Starter
Junior Member
 
Joined: Jun 2007
Posts: 411
Total Cats: 0
Default

Originally Posted by JasonC SBB
Here's a code snippet from ms2_extra_idle.c

tmp1 = IACmotor_pos - (((long)(Kp + Ki - Kd) * ...

Why is Kd subtracted from the others and not added??
This appears to be the inverted sign problem.
If it is inverted, then it does makes sense that the recommended d is zero!

I hope it really is as simple a fix as it looks. It does explain the apparent shift to the right of the sinusoids with high d.
Old Jun 13, 2011 | 10:27 PM
  #207  
bearda's Avatar
Junior Member
 
Joined: Apr 2010
Posts: 107
Total Cats: 0
Default

Originally Posted by JasonC SBB
What Ken said, "The D term is not coupled to error in the idle speed control loop, it's only coupled to change in RPM, which changes the sign you need to use to get the proper behavior." suggests that the code needs to be modified then, so that the sign is inverted, to get the proper behaviour.
I thought there was something odd about that code the first time I saw it. It doesn't incorporate the SP target at all, which makes me think it's not really acting like a normal PID control (and the sign inversion). Does anyone know why it it's just basing it on the difference between the past and present error terms instead of RPM alone? Is it so you get only the control loop induced rpm change instead of also incorporating a target change and possibly overshooting?

EDIT: Nevermind, I figured it out. If you assume the target is constant it cancels itself out of each first-order change calculation. Then you're just calculating the difference between the current stage error and the previous stage error.

Last edited by bearda; Jun 13, 2011 at 10:43 PM.
Old Jun 13, 2011 | 10:44 PM
  #208  
JasonC SBB's Avatar
Elite Member
 
Joined: Jul 2005
Posts: 6,420
Total Cats: 84
Default

Correct, the 'D' part of the algorithm only looks at the derivative of the RPM and not the derivative of the error. In the case of idle control, the target doesn't change very rapidly anyway, so it makes little difference.
Old Jun 14, 2011 | 02:52 AM
  #209  
Reverant's Avatar
Elite Member
iTrader: (10)
 
Joined: Jun 2006
Posts: 6,020
Total Cats: 369
From: Athens, Greece
Default

Give it a try then.
Attached Files
File Type: zip
ms2extra_3.1.1_PID_Kd.zip (808.0 KB, 37 views)
Old Jun 14, 2011 | 03:37 AM
  #210  
richyvrlimited's Avatar
Elite Member
iTrader: (1)
 
Joined: Jun 2006
Posts: 2,642
Total Cats: 42
From: Warrington/Birmingham
Default

Reverant:

1) Nice quick job!
2) Is that based on 'Mario b's' code changes?
Old Jun 14, 2011 | 03:55 AM
  #211  
Greg G's Avatar
Thread Starter
Junior Member
 
Joined: Jun 2007
Posts: 411
Total Cats: 0
Default

Yay! Thanks Dimitris! Will try it out as soon as I get a chance!

And yes, is this based on the official 3.1.1 or the mario b 3.1.1?
Old Jun 14, 2011 | 03:59 AM
  #212  
Reverant's Avatar
Elite Member
iTrader: (10)
 
Joined: Jun 2006
Posts: 6,020
Total Cats: 369
From: Athens, Greece
Default

I don't remember whether the 3.1.1 directory I edited was the vanilla or the mario_b version, however, I didn't find the "tmp1 = IACmotor_pos ..." snippet, instead I found "tmp1 = IACmotor100 ..." which leads me to believe that I had in fact, applied mario_b's patch to that directory.
Old Jun 14, 2011 | 05:44 AM
  #213  
Greg G's Avatar
Thread Starter
Junior Member
 
Joined: Jun 2007
Posts: 411
Total Cats: 0
Default

Well I tried it! No dice. :(

The idle valve would immediately go to 60.2 (maximum open duty) and stay there. Idle RPMs stayed at 2k. Even setting PID to 0 0 0 didn't lower the idle duty. Setting it to PWM warmup lowered the idle duty as preset (so I know it isn't stuck).

Thanks again for the modified file Reverant!

Originally Posted by Reverant
I don't remember whether the 3.1.1 directory I edited was the vanilla or the mario_b version, however, I didn't find the "tmp1 = IACmotor_pos ..." snippet, instead I found "tmp1 = IACmotor100 ..." which leads me to believe that I had in fact, applied mario_b's patch to that directory.
The snippet from Jason came from me- I sent him the file from the mario b version. So you may have modified the release version of 3.1.1. Could that have caused the high idle issue?

EDIT: Oh crap I forgot to point the TS project to the correct firmware directory after flashing, maybe that's the issue...I found it set to 3.1.0 >.<
Attached Files
File Type: msl
2011-06-14_16.29.58pid000.msl (852.3 KB, 144 views)
File Type: msq
2011-06-09_20.47.47idleve3.msq (79.6 KB, 139 views)

Last edited by Greg G; Jun 14, 2011 at 06:08 AM.
Old Jun 14, 2011 | 04:16 PM
  #214  
muythaibxr's Avatar
Junior Member
 
Joined: May 2007
Posts: 248
Total Cats: 0
From: Columbia, MD
Default

Originally Posted by Greg G
If it is inverted, then it does makes sense that the recommended d is zero!

I hope it really is as simple a fix as it looks. It does explain the apparent shift to the right of the sinusoids with high d.
Please see:

http://bestune.50megs.com/typeABC.htm

Specifically, I'm using type C.

Ken
Old Jun 14, 2011 | 05:18 PM
  #215  
Greg G's Avatar
Thread Starter
Junior Member
 
Joined: Jun 2007
Posts: 411
Total Cats: 0
Default

Thanks Ken! That is the controller output response that I'm trying to get from the idle valve, without much success.
Old Jun 14, 2011 | 05:21 PM
  #216  
Braineack's Avatar
Boost Czar
iTrader: (62)
 
Joined: May 2005
Posts: 80,552
Total Cats: 4,368
From: Chantilly, VA
Default

do the dual throttle body mod and then report back...
Old Jun 14, 2011 | 06:31 PM
  #217  
JasonC SBB's Avatar
Elite Member
 
Joined: Jul 2005
Posts: 6,420
Total Cats: 84
Default

Hi Ken,

I understand that the "type C" controller in the above link removes the setpoint information from the 'D' calculation. My hypothesis as to the problem has nothing to do with this.

What I see from ggermar's datalogs is that the effect of the D term (comparing 0, to medium, to large values of D), seems to have its sign inverted. That is, a dropping RPM seems to reduce the solenoid duty cycle, and vice versa, instead of the dropping RPM producing an increase in solenoid duty cycle. I see comparing the RPM and duty cycle sinusoids.

With a pure P controller the sinusoids will be inverted from each other (sinusoid of duty will be upside down compared to the sinusoid of RPM). That's because a dip in RPM produces a rise in duty. The D term should "mix in" a 90* phase shifted sinusoid. So as D is added, the duty cycle sinusoid should start "shifting left" as compared to the old P-only duty sinusoid. That is, the duty sinusoid should peak *before* the RPM has reached bottom. This is from the math:

-Derivative of a sinusoid is a 90* phase shifted sinusoid.

- If you add a 90* phase shifted sinusoid to a regular sinusoid, it looks like a sinusoid that's phase shifted between 0 and 90*.

- If you add an inverted, 90* phase shifted sinusoid, it phase shifts between 0 and -90* instead. (visually it appears to shift in the opposite direction)

What I see in Greg's datalogs is that the phase shift is *to the right* instead of to the left. This suggests that the D calculation is inverted (subtract instead of add, or vice versa). And the code snippet I posted, appears to be it. The P and I terms are added, but the D term is subtracted......

This is why practical D circuits in electronics are called "phase lead" circuits. Its sorta kinda like leading the clay pigeons when you shoot 'em.

Cheers.

Last edited by JasonC SBB; Jun 14, 2011 at 07:56 PM.
Old Jun 14, 2011 | 06:42 PM
  #218  
JasonC SBB's Avatar
Elite Member
 
Joined: Jul 2005
Posts: 6,420
Total Cats: 84
Default

Originally Posted by Braineack
do the dual throttle body mod and then report back...
You don't think trying code improvements is easier?

Wouldn't it be cool if the MS were the only ECU that could idle an intercooled s/c without a dual TB?
Old Jun 14, 2011 | 07:40 PM
  #219  
Braineack's Avatar
Boost Czar
iTrader: (62)
 
Joined: May 2005
Posts: 80,552
Total Cats: 4,368
From: Chantilly, VA
Default

I have no issue with the code. but i don't have crazy throttled volume.

problem is, when they went to the ms3 they made a huge improvement with the a/c idle up adder...and they really have no motivation to backport things to older hardware and would like to move forward with ms3 features and improvements.
Old Jun 14, 2011 | 07:59 PM
  #220  
Greg G's Avatar
Thread Starter
Junior Member
 
Joined: Jun 2007
Posts: 411
Total Cats: 0
Default

I just feel the MS2extra code is very close to done. And it's very good. Maybe just a final update incorporating the mario b patch, and hopefully a solution to the D issue, and it'll be perfect. The data collected does show that the sinusoid moves to the right, not left. So there is something there not working properly...

I understand the focus is on MS3, but solving the PID issue will benefit MS3 as well.



All times are GMT -4. The time now is 06:06 PM.