My Car tried to Kill me today (1973 Beetle +SDU Conversion), Potential Cruise Control Bug ?

[Pete9008]

I've already made my suggestions above but in case it wasn't clear enough.
Short term:

• Reduce throttle availability in cruise mode. The default in the param_prj.h is 50%, on something like an SDU that is way too much. You need just enough to maintain cruise at the speeds, and on the gradients, you are likely to encounter. 10-20% would be more like it. If people find cruise is underpowered they will increase it but if it just works they are likely to just run with defaults so the defaults should be sensible for all cases. In no case should cruise be allowed enough throttle authority to come anywhere near over riding the brakes on the driven axle (particularly relevant for RWD cars which tend to have weaker brakes on the driven axle)!
• Fix the bug in the scheduler that allows events to be missed. It's unlikely to be the problem here but it is still a bug.
• Make going into neutral, or a change in direction, cancel cruise. Again it's not a fix but it adds options if the problem occurs again and more importantly it adds a degree of redundancy on a potentially safety critical function that otherwise doesn't have any.

These won't fix the problem but should help mitigate the consequences.

Longer term:

• Implement, and encourage, logging or some other way of collecting data (I really don't care what it is but you need the data!). Without it you will end up back in exactly the same position in another 6months. Agree completely with comments on root cause but without data your chance of finding it is low! That's why the simulator was able to find and fix issues, it generated the data, the data generated the understanding and the understanding provided the fixes.
• Review the code (I intend to restructure the way the scheduler and CAN threads work on my port as I'm not happy with the way it is at the moment, if this turns anything else up I'll post it here).

[johu]

On it.
And wondering whether to disallow cruisemode button plus disallow cruise mode all together in "DefaultForward" dirmode.

Then I'd have the loss of din_forward cancel cruise control. It means for all people who permanently run their drive train in reverse they must properly set up dirmode as "SwitchReversed" or "ButtonReversed" instead of physically reversing the signals. If cruise control is used, that is.

[Pete9008]

That would make sense, makes for a nice simple implementation in the cruise control code.

[uhi22]

I have no clue about the details, but nevertheless some thumb questions, maybe they help
1. What happens, if some of the parameter bits, which are configuring the throttle, are unintentionally modified, e.g. by an out-of-bounds array access or just a bit flip in RAM? Are these parameters "repaired" somehow cyclically? Or only loaded on power-on?
2. I saw that in case of dual channel potmode, if one of the channels is not plausible, just the other is used. I would have assumed that the pedal is only treated as valid if both channel fit together. (Maybe I miss some puzzle parts).
3. I'd propose to introduce some kind of "limp home mode", with highly reduced torque and speed, for the case that the software finds inconsistent things. And to actively check for inconsistencies, e.g. inconsistencies in parametrization (redundant storage) and inconsistencies in signals (poti1 versus poti2, or accelerator pedal versus brake pedal, ...)
4. What happens, if the port configuration of the brake input signal or the throttle is unintentionally modified, e.g. from "input" to "output" or similar? Is this detected? When does this heal?

[Pete9008]

Some good points there

I'd quite like to have most of the configuration parameters kept in flash, so that they are effectively read only, and only allow them to be updated when the inverter is not running. It's a big change though and also looses a lot of the current flexibility which is very nice to have when developing/debugging/setting up/commissioning. Maybe it would be possible to have a development mode with them in SRAM and a normal mode where they are in FLASH?

Having said that I'm reasonably comfortable with them in SRAM, the way parameters are currently accessed is fairly robust (mostly via constant indexes) but that doesn't prevent stray pointers elsewhere in the code from messing them up. Also if you start to lose confidence in SRAM what do you do about every other variable, the stack, stack pointer, etc. Look up SEUs and ways of dealing with them, it gets very complex very quickly if you want to do it properly! Regularly scanning parameters against the FLASH copy and lighting the MIL if they differ could be a good compromise? Sense checks on inputs also make sense but only if you can come up with a robust set of tests and without some redundancy that's not easy.

Agree on the dual channel potmode. The decision is what to do if they disagree, I've gone for using the lower of the two values and again lighting the MIL (redundancy is pointless if you don't know you have lost a layer) but probably should scale it down too. I'm not a big fan of limp modes but could probably be persuaded; I'd prefer to light the MIL and leave it to the driver to decide the best course of action.

[johu]

Thanks for that

I have no clue about the details, but nevertheless some thumb questions, maybe they help
1. What happens, if some of the parameter bits, which are configuring the throttle, are unintentionally modified, e.g. by an out-of-bounds array access or just a bit flip in RAM? Are these parameters "repaired" somehow cyclically? Or only loaded on power-on?

They wouldn't be, only loaded at power-up. Some are modified over CAN if so configured. Could sortof explain the runaway in my case but not in 7yatnas - his cruise control diligently regulated speed so no real runaway there. There's no parameter that would cause cruise control to not stop on brake signal.

2. I saw that in case of dual channel potmode, if one of the channels is not plausible, just the other is used. I would have assumed that the pedal is only treated as valid if both channel fit together. (Maybe I miss some puzzle parts).

It is sort of hard to define a safe state for a car in traffic. Loss of power can put you in a dangerous situation also, that's why I decided to work with the remaining channel if one is gone.

3. I'd propose to introduce some kind of "limp home mode", with highly reduced torque and speed, for the case that the software finds inconsistent things. And to actively check for inconsistencies, e.g. inconsistencies in parametrization (redundant storage) and inconsistencies in signals (poti1 versus poti2, or accelerator pedal versus brake pedal, ...)

That is probably an answer to number 2. Just reduce power to less harmful levels if things go out of check. Could also be a requirement that the brake pedal must be depressed once at power up to at least check at that instant that the signal is firing.

4. What happens, if the port configuration of the brake input signal or the throttle is unintentionally modified, e.g. from "input" to "output" or similar? Is this detected? When does this heal?

it wouldn't heal. So you're thinking a function DigIo::CheckConfiguration() that checks if the pin is still in the desired mode periodically?

I'd quite like to have most of the configuration parameters kept in flash, so that they are effectively read only, and only allow them to be updated when the inverter is not running. It's a big change though and also looses a lot of the current flexibility which is very nice to have when developing/debugging/setting up/commissioning. Maybe it would be possible to have a development mode with them in SRAM and a normal mode where they are in FLASH?

That faintly struck me as well. Have a commissioning mode that retains the current behaviour and in production mode periodically check parameters that are not CAN mapped against their saved values.

Agree on the dual channel potmode. The decision is what to do if they disagree, I've gone for using the lower of the two values and again lighting the MIL (redundancy is pointless if you don't know you have lost a layer) but probably should scale it down too. I'm not a big fan of limp modes but could probably be persuaded; I'd prefer to light the MIL and leave it to the driver to decide the best course of action.

Yes, the MIL has always been there, provided you wired it up/mapped it.

In some standards you are required to periodically check RAM consistency and even registers. But we are talking trains and airplanes here. It's not super hard, I've seen it. You've got some high priority interrupt in which you check like a byte of SRAM with a few patterns and then quickly return. It's designed to check the entire RAM within a few hours.

I'm afraid we won't catch the issue at hand that way though...

[Gregski]

I'm just here cause someone came up with a better thread title than one of mine !

but I luv these tow shackles

20221228_134904 arrows.jpg (43.34 KiB) Viewed 4123 times

[Pete9008]

I'm just here cause someone came up with a better thread title than one of mine !

That is probably an answer to number 2. Just reduce power to less harmful levels if things go out of check. Could also be a requirement that the brake pedal must be depressed once at power up to at least check at that instant that the signal is firing.

You got me wondering wherther that is why having the brake pressed when starting is becoming so standard on modern cars?

In some standards you are required to periodically check RAM consistency and even registers. But we are talking trains and airplanes here. It's not super hard, I've seen it. You've got some high priority interrupt in which you check like a byte of SRAM with a few patterns and then quickly return. It's designed to check the entire RAM within a few hours.

I'm afraid we won't catch the issue at hand that way though...

Yea but that's only a tiny fraction of the full system implementation

Agree though, don't think it will provide benefits in this case.

[arber333]

....

In some standards you are required to periodically check RAM consistency and even registers. But we are talking trains and airplanes here. It's not super hard, I've seen it. You've got some high priority interrupt in which you check like a byte of SRAM with a few patterns and then quickly return. It's designed to check the entire RAM within a few hours.

I'm afraid we won't catch the issue at hand that way though...

In aviation we call it the BITE test. It is a maintenance action which requires to be recorded. After you would get an alert during the preflight or during flight or even in scheduled maintenance you would turn on the particular device (on ground) and select the Built In Test which would quickly (debatable) go through every conceivable input scenario and observe the output values. They would be compared to the table of veted results. Any negative outcomes would be recorded and displayed for technical staff to then go deeper into their maintenance process.
It is a lot of help with those black box components that normal people (i mean licenced technicians) dont know how they work on the inside at all...

Assumption here then is that design of those components was subjected to so many cycles of testing before use that robustness 10-9(extremely improbable) was demonstrated just by bringing repeatable results as evidence. I think that DIY EVs like UAVs for that matter are still in the 10-5(remote) in robustness category.
System safety: https://en.wikipedia.org/wiki/AC_25.1309-1

Much was improved by adapting OEM components but we still have to learn a lot. Online recording here will help a lot. But since we dont know which parameter to observe we need to record ALL of them and correlate them to the timeline.

Edit: Once i was present at similar test of the Black Box (CVR) and we found out one of the microphones was offline. When we investigated further we found out mic was burned out. Later we found out it was not installed correctly and could cause some panels to overload and burnout... luckily the hero mike sacrificed itself and saved components downstream . I am not sure why they were allowed to certify component which could be incorrectly installed on airplane in the first place...

[7yatna]

I'm just here cause someone came up with a better thread title than one of mine !

but I luv these tow shackles


20221228_134904 arrows.jpg

two shackles that will give you a buzz if you pull hard.

i used the original cables without modifications. I gotta tell you something gregski I used those modules based on your recommendation and experience i even bought two of the eBay's listings you reference in your thread.

cheers

[johu]

I have made some changes and will test them today. They are of safety type, so the software becomes slightly harder to use and some flexibility is removed.

• There is a now an upper limit to cruise control power (cruisethrotlim), defaulting to 50%
• Cruise mode "Button" has been removed. You now need to 2 position switch and when it's off, cruise control cancels as well (redundancy)
• if din_forward is taken away cruise control is also cancelled (reverse cruise control seems a bad idea anyway). This has some consequences that might require rewiring:
  • Cruise control won't work in DefaultForward dirmode, unless you also set din_forward
  • Cruise control won't work if you set din_reverse for going forward, e.g. on Tesla drivetrains - instead set din_forward and select dirmode=SwitchReversed
• In cruise mode CAN din_cruise must be set in addition to a non-zero value in cruisespeed (din_cruise times out after 500ms of CAN comms loss)
• I have made extra sure that Neutral is neutral by adding "if (Param::GetInt(Param::dir) == 0) finalSpnt = 0" at the end of ProcessThrottle() (could be related to a report by Bratitude)
• Scheduler fix

I think I'll also add the brake pedal check on start

UPDATE: I think I won't make it a hard requirement to have the brake pedal pressed at start. If cruisemode=Off I will forego the check. What do people think?

[johu]

Implement, and encourage, logging or some other way of collecting data

As a low entry barrier method I thought about some sort of spot value dump that is saved to flash on request and can be dumped out back home. We can't rely on error states here so it'd have to be some user request when the car behaves weirdly. Not sure what could be a good trigger pattern though.

[Pete9008]

Sounds good

• There is a now an upper limit to cruise control power (cruisethrotlim), defaulting to 50%

I was assuming that this was what idlethrotlim did but it's not is it?

Would it be worth adding another parameter for this that defaults to say 20%? My reasoning is that on a FWD car, even with a reasonably powerful motor, the front brakes should easily be able to stop the motor regardless of the limit. On a car with a lower power motor 50% might not be enough throttle authority to maintain speed. On a powerful RWD car the rear brakes may struggle to hold at 20% throttle so the 50% value could still be iffy. A fixed value will never be right for all cases. Having a configurable one that defaults to say 20% should be safe in pretty much all cases out of the box but the user can increase if it's needed and safe.

Edit - just twigged cruisethrotlim is already a parameter isn't it! I'd still drop the default down to 20% though.

• Cruise mode "Button" has been removed. You now need to 2 position switch and when it's off, cruise control cancels as well (redundancy)

I'd probably keep the button as long as the level is such that a wire break would put it in the off state.
Not over keen on a switch. Does it means that cruise control will re-engage as soon as you come off the brake (which could come as a surprise!) or does it only engage when the switch changes changes state? Redundancy in turning it off is the critical bit.

• if din_forward is taken away cruise control is also cancelled (reverse cruise control seems a bad idea anyway). This has some consequences that might require rewiring:
  • Cruise control won't work in DefaultForward dirmode, unless you also set din_forward
  • Cruise control won't work if you set din_reverse for going forward, e.g. on Tesla drivetrains - instead set din_forward and select dirmode=SwitchReversed
• In cruise mode CAN din_cruise must be set in addition to a non-zero value in cruisespeed (din_cruise times out after 500ms of CAN comms loss)
• I have made extra sure that Neutral is neutral by adding "if (Param::GetInt(Param::dir) == 0) finalSpnt = 0" at the end of ProcessThrottle() (could be related to a report by Bratitude)
• Scheduler fix

Like all those

I think I'll also add the brake pedal check on start

UPDATE: I think I won't make it a hard requirement to have the brake pedal pressed at start. If cruisemode=Off I will forego the check. What do people think?

I'd probably include, foot on the brake has pretty much become a habit for me when staring these days and given the importance of the brake, not just in cruise mode, it seems sensible.

Edit - One other thought regarding the failure mode for the above. We're assuming that the cruise control was still running because the motor seemed to pull harder as it was slowed down. Is this necessarily the case? Could it be that the cruise algorithm wasn't running and just stuck at fixed throttle and the apparent increase in torque was due to the inherent behaviour of the AC induction motor (due to an increase in induced rotor current when slowing)?

[bexander]

To my knowledge many OEM cars have a lower speed limit that is needed to be exceeded before you can activate cruise control and if the car drops in speed under this limit the cruise control drops out completely. Don't know if this is included in the openinverter cruise control. If not, it would add another layer of safety.

[uhi22]

I think I'll also add the brake pedal check on start

UPDATE: I think I won't make it a hard requirement to have the brake pedal pressed at start. If cruisemode=Off I will forego the check. What do people think?

Are there cars around which can be started without pressing the brake pedal? For me it's a natural habbit since more than 30years, to push the brake (and also clutch, if present) when starting. My Ioniq uses the brake to distinguish between "user wants just ignition on" (if brake is not pressed) and "user wants the drive mode" (if brake is pressed). So yes, brake would be a normal condition to enter drive mode.

[johu]

Well yes, thing is.... some people don't even connect it

re Pete:
idlethrotlim is just for hill hold and idle speed
Mode "button" means you press a button to start cruise control and press the brake to stop it. No redundancy unless you also have Fwd/reverse switch. Mode switch means you engage cruise control by turning a switch on (edge triggered) and turn off by turning the switch off or pressing the brake pedal. To start it again you need to turn the switch off and on again

[muehlpower]

it is certainly not a problem to press the brake to establish "Fahrbereitschaft"(ready to drive) as it is officially called in Germany, but with my manual gearbox cars i can always start the engine without pressing anything and with the automatic cars the "P" or "N" position of the shifter is sufficient. And not all are older than 30 years

[johu]

I could as well release it as I tested in on both cars but thought I'd post here first.

Changes as above:

• There is a now an upper limit to cruise control power (cruisethrotlim), defaulting to 50%
• Cruise mode "Button" has been removed. You now need to 2 position switch and when it's off, cruise control cancels as well (redundancy)
• if din_forward is taken away cruise control is also cancelled (reverse cruise control seems a bad idea anyway). This has some consequences that might require rewiring:
  • Cruise control won't work in DefaultForward dirmode, unless you also set din_forward
  • Cruise control won't work if you set din_reverse for going forward, e.g. on Tesla drivetrains - instead set din_forward and select dirmode=SwitchReversed
• In cruise mode CAN din_cruise must be set in addition to a non-zero value in cruisespeed (din_cruise times out after 500ms of CAN comms loss)
• I have made extra sure that Neutral is neutral by adding "if (Param::GetInt(Param::dir) == 0) finalSpnt = 0" at the end of ProcessThrottle() (could be related to a report by Bratitude)
• Scheduler fix
• Brake pedal must be seen at startup when cruise control is enabled

If you routinely want to check for the brake pedal even without cruise control, you can set cruise control to "switch" or "CAN" and just not wire it up.

[uhi22]

Some more ideas regarding plausibility checks:

Checksum over the fix configuration parameters: Over the parameter block, which only change during the development phase and never change during vehicle run, calculate cyclically (e.g. once per second) a CRC32. Compare the calculated CRC32 with the expected value, which is also stored as parameter. In case of mismatch, set LimpHomeReason = LIMPHOME_REASON_PARAMETRIZATION_CORRUPTED.

Register Monitoring: Identify all configuration registers of the controller, which could influence a safety relevant functionality (e.g. port config registers, clock configuration registers, CAN configuration registers, ...). Cyclically (e.g. once per second) compare the actual value of the register with the expected value. In case of mismatch, set LimpHomeReason = LIMPHOME_REASON_REGISTERMONITORING.

Canaries: Between the normal "global variables", add variables which are set to a certain value during startup, and which are never written afterwards. Cyclically compare the values of these variables with the expected value. In case of mismatch, set LimpHomeReason = LIMPHOME_REASON_CANARY_CORRUPTED. Especially interesting are the variables which are not cyclically overwritten anyway, they have the risk that an unwanted modification does not heal. The idea of the canaries is, that if memory is overwritten e.g. by wrong pointer, there is a chance that this overwriting does not only modify the "productive" variable, but also overwrites the canary in the neighborhood, so that the software is able to detect the situation.

Use the LimpHomeReason != LIMPHOME_REASON_NONE to apply hard-coded limits for torque and speed, and make a blink-code on the motor-error-lamp, to give the user a chance to have some more information than "something went wrong".

Declare LimpHomeReason at least with 32 bits, and the initial value LIMPHOME_REASON_NONE an unlikely pattern, so also a corruption of this variable has a good chance to be detected.

Finally, send the LimpHomeReason in a developer message on CAN, so that there is the chance to see it in the logging.

[johu]

I could calculate the CRC over the GPIO, ADC and timer registers and accordingly add a function to Anain:: and DigIo:: that triggers a recheck and returns the result. Should be pretty easy. By "canaries" you mean such things as the many variables in Throttle:: ?

[uhi22]

To be honest, I have no idea where to place the canaries exactly. Only the general rule, next to the important variables. This may be a few or may be a lot. The brute force canary style would be, to add one after each regular variable, in each of the safety relevant classes.

[Pete9008]

Not a huge fan of adding lots of code to try and increase reliability - worked on a system in the past when a stray pointer in the error logging code caused the system to intermittently crash, took ages to find and the error logging code had never even been used for diagnostics in the field!

If you want to protect parameters I'd prefer to access them directly from flash in normal operation, little slower due to the slower flash read time but gives read only variables without the code overhead. For configuration mode, mirror them to ram, modify them there and then copy back to flash if saved. I'm going to give this a try on my port and see what the performance hit is.

To protect registers there are lock functions built into the F103 hardware, e.g. the LOCK bits (in TIMx_BDTR) in the timers or the port configuration lock register (GPIOx_LCKR) for GPIO. There may be others but I've not had a thorough look. A function called after initialisation is complete (but only if a LOCK parameter is set to true) would prevent reconfiguration again without much extra code.

If you do want to add some diagnostic code then a stack pointer watermark check would be the first thing I'd add. Save the stack size to a parameter within the highest priority interrupt (update only if larger than the parameter value). Reading, or logging the parameter will then tell you whether the stack is in danger of overflowing.

If you do go the the LimpHomeReason route I'd suggest using some of the bits (or another parameter) to record a little more detail about the problem, for example which canary was corrupted, which register was wrong or which parameter wrong (difficult with a CRC but possible if you check against flash)

[johu]

Would appreciate if people tested the above Then I will make it a release.

Will also look into the register locking, that seems a low complexity path to prevent weirdness

[E46Driver]

I just saw this thread today and decided to reply. I experienced similar issues when trying to calibrate the throttle. I was not using cruise control, but my symptoms were the same - so perhaps one more thing to review. Also I am on old software Version 5.23R.

Details are as follows: Trying to set up throttle using BMW 5V signal read by Arduino, modified to simulate logarithmic response, then output to OI.

I recall having my OI potMax parameter set to 3030. Which would equate to about 2.5V. Of course if the throttle was pressed hard enough to exceed this voltage - as soon as the voltage went above this, OI would shut down PWM and the motor would coast to a stop. When throttle was slowly released and voltage dropped below 2.5V, motor would instantly accelerate (to what OI thought was full throttle again).

Curious thing was that on random cases, it seemed like the throttle "stuck" and the motor would continue accelerating, instead of stopping after 2.5V. At the time I discounted it as stupid settings on my part --- ie --- potMax should have been 4095 since I was feeding the analog signal with 3.3V for max throttle. This testing was not done on the road (vehicle was up on blocks). But reading this thread got me wondering - could something possibly saturate the throttle input? Or the code managing the throttle (torque) demand calculation? And how scary would it be to have the vehicle acting on its own and not responding to your inputs! YIKES!

You guys are often discussing things way above my understanding. And maybe this is all not even relevant. But thought I'd mention it in case you have some other ideas --- because safety is always of utmost importance!

I should note that I have not seen this issue AT ALL since I finalized my throttle curve and adjusted potMax to 4095.

[Bigpie]

My issue has also not resurfaced since I've corrected my can bus resistance, though if someone doesn't tackle adding a throttle specific can timeout, it's something I plan on doing once I've got my bench test setup properly.