Cruise control #12

jima72 · 2021-02-01T00:26:58Z

Initial commit, will definitelly need fixes.

Initial release of Cruise Control

cesarBLG · 2021-02-05T08:46:12Z

Source/Orts.Simulation/Simulation/RollingStocks/SubSystems/CruiseControl.cs

+                        {
+                            //Simulator.Confirmer.Information(Locomotive.Train.AccelerationMpSpS.SmoothedValue.ToString());
+                            // 
+                            if (SpeedRegMode == SpeedRegulatorMode.AVV)


I would prefer if this block was moved to a TCS script, keeping in this class a more generic Automatic Train Operation (ATO) system.

I suggest the TCS-ATO interface to be like this:

void SetATOTargetParameters(float targetSpeedMpS, float targetDistanceM)

I think the targetDistanceM is necessary to accurately stop a train in a platform when required, but I might be wrong.

This should not be there, it was only a testing part. Will remove it. We can work on ATO later.

cesarBLG · 2021-02-05T08:49:57Z

Source/Orts.Simulation/Simulation/RollingStocks/SubSystems/CruiseControl.cs

+            controllerTime = inf.ReadSingle();
+            CurrentSelectedSpeedMpS = inf.ReadSingle();
+            currentThrottlePercent = inf.ReadSingle();
+            float deltaCoefficient = inf.ReadSingle();


What's the purpose for saving this parameters but then discard them?

Will review.

Usually the only things that should be saved are those that change during the simulation (i.e. selected cruise speed, cruise speed operation mode, etc.)

Fixed in new PR

You don't need to create more PRs. When you push the changes to your upstream repository, this PR is automatically updated.

…and MultiPositionControl initial commit; by Jindrich

cesarBLG · 2021-02-06T08:42:32Z

Source/Orts.Simulation/Simulation/RollingStocks/MSTSLocomotive.cs

            WaterScoopWidthM = locoCopy.WaterScoopWidthM;
            MoveParamsToAxle();
+            if (locoCopy.CruiseControl != null)
+                CruiseControl = locoCopy.CruiseControl;


I believe you have to perform a deep clone of the CruiseControl class, not just copying the reference. The Copy() method is called when there are several instances of the same engine in the consist, to avoid parsing the STF file more than once.

Will do Copy() method for the CruisControll class. What variables should the copy method include?

The Copy() method is a substitution for Parse(STFReader stf), so you should set the same variables as in the CruiseControl.Parse() method. For example, UseThrottleAsSpeedSelector, MaxAccelerationMpSS, etc.

cesarBLG · 2021-02-06T08:46:58Z

Source/Orts.Simulation/Simulation/RollingStocks/MSTSLocomotive.cs

+        public TrainType SelectedTrainType = TrainType.Pax;
+        public void ChangeTrainTypePaxCargo()
+        {
+            SelectedTrainType = SelectedTrainType == TrainType.Pax ? SelectedTrainType = TrainType.Cargo : TrainType.Pax;


Could you provide more information about this selector? Is it only used for cruise control (in real trains), or should it change also braking related parameters? I was planning to add something similar for brake operation.

I may write large story about brakes. Because the brakes are still simulated same as back when we only have MSTS. Not a very large, challenging or breathtaking information. But this needs complete review (I mean the brakes) or small steps for us, until we are on the Moon, (or Mars nowadays).
The TrainType cargo : pax is now used for cruisecontrol only to select different time in reduced force, before the whole train is in tension on all couplers.

Thanks, let's keep the button for cruise control only then.

cesarBLG · 2021-02-06T08:49:41Z

Source/Orts.Simulation/Simulation/RollingStocks/MSTSLocomotive.cs

+                            break;
+                        }
+                        bool metric = cvc.Units == CABViewControlUnits.KM_PER_HOUR;
+                        float temp = CruiseControl.RestrictedSpeedActive ? MpS.FromMpS(CruiseControl.CurrentSelectedSpeedMpS, metric) : temp = MpS.FromMpS(CruiseControl.SelectedSpeedMpS, metric);


Suggested change

float temp = CruiseControl.RestrictedSpeedActive ? MpS.FromMpS(CruiseControl.CurrentSelectedSpeedMpS, metric) : temp = MpS.FromMpS(CruiseControl.SelectedSpeedMpS, metric);

float temp = CruiseControl.RestrictedSpeedActive ? MpS.FromMpS(CruiseControl.CurrentSelectedSpeedMpS, metric) : MpS.FromMpS(CruiseControl.SelectedSpeedMpS, metric);

Just a little style change

I was very tired to write this :D

cesarBLG · 2021-02-06T08:58:01Z

Source/RunActivity/Viewer3D/Signals.cs

                        if (mstsSignalType.Semaphore)
                            glowDay = 0.0f;
+                        /* Jindrich
+                         * Why is this here? I want all my lights glowing, when I have glowing on


As I see, you have created a different PR for this. To avoid merge issues, you should remove this line from this PR.

Probably this block should be kept for backwards compatibility anyway. I think you can manually force SHUNTING and INFO signals to glow using the sigcfg.

This PR was cancelled.

… classes

Csantucci · 2021-02-16T08:17:26Z

Just to let you know how I am proceeding, I get Jindrich's updates from this PR or from PR #13 (I see that Jindrich updates both PRs in parallel) and apply them to my cruise-control branch (which is the base of PR #13), which I push to my remote repository request. I have too removed the signal glow changes.
I proceed this way because usually Jindrich's updates to PR #13 usually generate merge conflicts. If it wouldn't generate merge conflicts, this workaround wouldn't be necessary.

cesarBLG · 2021-02-16T08:59:41Z

I believe the merge conflicts are caused because of the two PRs. Both are referencing the same remote branch (jima72:CruiseControl) but they compare against different branches in your repo. You can see that commits in both PRs have the same hash.

As you are using 'git apply' instead of 'git merge' to apply Jindrich's changes, the commit information is lost and git is unable to know that your diff and Jindrich changes are the same.

If you wish to modify the PR locally, without loosing commit information, you can follow this instructions: https://docs.github.com/en/github-ae@latest/github/collaborating-with-issues-and-pull-requests/checking-out-pull-requests-locally#modifying-an-active-pull-request-locally

Csantucci · 2021-02-16T11:12:43Z

Thanks César, I resolved the conflicts with your hint. I hope I did things OK.

cesarBLG · 2021-02-19T13:44:20Z

Source/Orts.Simulation/Simulation/RollingStocks/MSTSLocomotive.cs

+                if (!IsPlayerTrain || CruiseControl.SpeedRegMode == CruiseControl.SpeedRegulatorMode.Manual || CruiseControl.UseThrottle)
+                    UpdateMotiveForce(elapsedClockSeconds, t, AbsSpeedMpS, AbsWheelSpeedMpS);
+                else if (IsPlayerTrain || CruiseControl.SpeedRegulatorOptions.Contains("engageforceonnonzerospeed"))
+                    CruiseControl.Update(elapsedClockSeconds, AbsWheelSpeedMpS);


Please have a look at this lines:
If train is not a player train, the first clause will be executed, which I believe it's OK
However, if train is player train, the 'else if' condition is unnecessary, so it could be replaced with an 'else'. Did you mean something else?

cesarBLG · 2021-02-19T13:48:41Z

Source/Orts.Simulation/Simulation/RollingStocks/MSTSLocomotive.cs


        public void StartThrottleIncrease()
        {
+            if (CruiseControl != null)


It would be better if you moved this to the end of StartThrottleIncrease() method, so the combined controls can be used normally, and the cruise control code only swallows the command when the combined control is in throttle positions.
The same applies for all Start/Stop Throttle Increase/Decrease methods

Well, some locos uses the throttle handle as CruiseControl mode selector (i. e. neutral (no force), drive (force until speed reached), confirmation for drive (need to be held until given speed) when in automatic mode. Then the lever in fact is not controlling the throttle anymore, and the logic for brakes and force is completely driven by cruise control. So in that case is necessary only the cruise control to be executed and the rest is skipped completely.

How does the throttle handle behave when the cruise control is disconnected? If it is just a single throttle lever (without combining brakes), then the changes that I suggest aren't affected. Could you please provide more accurate description of the throttle lever on these locos?

I have in mind some locomotives with a combined control lever, where the throttle acts as a speed selector, but when the lever is moved to a braking position, the brakes are applied and the cruise control is disconnected.

Imagine you have a switch, with positions M and A. It the switch is in M position, the lever (plus/neutral/minus) works as a throttle - cruise control disconnected. If it's set to A, then the cruise control is active, but the train will not move, until you momentarily set the throttle lever to plus. The cc will then set the throttle virtually to 100%, and the force is maintained internally by overriding the UpdateMotiveForce. So the throttle is set to 100%, but the force to the loco is internally computed by cc.

Maybe wait, until I upload the new MultiPositionController class, with wich you can stack up any controller with predefined steps. These are also bound to cruise control if that iv available. The variety of controls aroud the globe is large, so I decided to do it this way, because the normal throttle lever or combined lever were not sufficient.

Here is a sample of such a controller:
ORTSMultiPositionController (
Positions (
Position ( ThrottleIncrease SpringLoadedBackwardsImmediatelly "Souhlas" )
Position ( Drive CruiseControl.NeedIncreaseAfterAnyBrake "Jízda" )
Position ( Neutral Default "Výběh" )
Position ( DynamicBrakeIncrease SpringLoadedForwards "EDB" )
Position ( TrainBrakeIncrease SpringLoadedForwards "Brzda" )
Position ( EmergencyBrake Null "Rychlobrzda" )
)
)

jima72 and others added 8 commits January 31, 2021 23:03

Update Signals.cs

71b801c

CruiseControl

3742786

Initial release of Cruise Control

Initial Cruise Control commit, by Jindrich

46140c3

Updates, fixes

7068916

Cruise control: first updates and fixes, by Jindrich

ba53341

Make object load only when needed

0b1807f

Changes to 464 + MultiPositionControler initial commit

f8536bd

Merge branch 'OR_NewYear_MG_NoWD' into cruise-control

52e34d1

cesarBLG reviewed Feb 5, 2021

View reviewed changes

Make Cruise Control object loaded only when needed; changes for E464 …

d11c39a

…and MultiPositionControl initial commit; by Jindrich

cesarBLG reviewed Feb 6, 2021

View reviewed changes

jima72 and others added 9 commits February 6, 2021 22:01

Update CruiseControl.cs

dcadfaa

Avoid crash in command list when cruise control not available

f7485dd

Various cruise control algorithm updates

6ec849d

Remove duplicate line in cruise control restore

f7ee526

Compact CruiseControl data into CruiseControl and CruiseControlViewer…

25a3119

… classes

Update CruiseControl.cs

8e0314f

Revert CruiseControlViewer introduction

8cae7e1

Merge branch 'OR_NewYear_MG_NoWD' into cruise-control

b11091e

Cruise control fixes, by Jindrich

cf767a1

Merge branch 'cruise-control' into CruiseControl

8d0e5a5

cesarBLG reviewed Feb 19, 2021

View reviewed changes

Csantucci merged commit 8d0e5a5 into Csantucci:OR_NewYear_MG_NoWD Jun 27, 2021

	float temp = CruiseControl.RestrictedSpeedActive ? MpS.FromMpS(CruiseControl.CurrentSelectedSpeedMpS, metric) : temp = MpS.FromMpS(CruiseControl.SelectedSpeedMpS, metric);
	float temp = CruiseControl.RestrictedSpeedActive ? MpS.FromMpS(CruiseControl.CurrentSelectedSpeedMpS, metric) : MpS.FromMpS(CruiseControl.SelectedSpeedMpS, metric);

Cruise control #12

Cruise control #12

Uh oh!

Conversation

jima72 commented Feb 1, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Csantucci commented Feb 16, 2021

Uh oh!

cesarBLG commented Feb 16, 2021

Uh oh!

Csantucci commented Feb 16, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jima72 Feb 19, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jima72 Feb 19, 2021 •

edited

Loading