spec/Vehicle: Add AccessoryOn signal

[komasayuki]

I could not find Accessory (ACC) signal in the VSS.

ACC is a common signal that is turned on before Ingnition.

https://cars-care.net/what-is-acc-in-a-car/
https://mservice411.com/c/ignition-switch

I propose adding an AccessoryON because some car have separate Ignition and Accessory signals in CAN.
And the signals are often next to each other in CAN.

[SebastianSchildt]

Thank your for proposing this PR.

Not directly opposed, but my question to the community is: Is it "worth"putting it in as it is probably only for old cars? Are there use cases, where such a signal might be useful? Nowadays maybe you just press start when you key is inside the car, or just switch into gear with your phone nearby. Not so many mechanical "turn your key" kind of cars.

On the other hand the the semantics might still be there, maybe the radio and AC already switches on when a person is detected in the seat. In case this goes in, should not be too hard to implement by vendors, just setting another bool.

[komasayuki]

@SebastianSchildt
Thank you for your comment!

The Accessory signal is used even in push-start vehicles and is also present in CAN. A lot of ECUs and devices in the vehicle work by using the Accessory signal.

In the case of push-start vehicles, the Accessory signal is turned on by pressing a push-start button without stepping on the brake.

[erikbosch]

Look ok for me

[danielwilms]

Signal might be of use, naming seems to be standard. Let's keep it open to review in the upcoming week.

[gunnarx]

How about this instead:

Remove the "boolean" signals and don't add even more boolean ones. I have received indication that systems have a single signal that specifies the different modes in one signal.

So new proposal

1. Remove (deprecate) IgnitionOn
2. Don't add a separate AccessoryOn
3. Replace with one signal that covers all:
   IgnitionStatus with 4 possible values (enum): Off, Accessory, Run, and Start.

[gunnarx]

See other comment for a better proposal

[erikbosch]

The important part as I see it is to have a clear definition of the signals. I am currently not sure if the proposed signal AccessoryOn is intended to cover only the state when the (possibly virtual) key is in state "ACC", or if it is intended to cover a functional state that includes multiple "key positions" (e.g. ACC+Run+Start)

If it just is supposed to reflect the (possibly virtual) key position then it is no problem to only have an enum, as AccessoryOn and IgnitionOn never will be true at the same time. If AccessoryOn might be true for multiple (possibly virtual) key positions then it still might be OK to only have the enum, if the conversion is well defined by implicit industry standard/praxis, possibly like:

AccessoryOn = (IgnitionStatus!=Off)
IgnitionOn = (IgnitionStatus==Run) OR (IgnitionStatus==Start)

Then a VSS client can just by looking at IgnitionStatus deduct the value of AccessoryOn. But if the mapping is vehicle/OEM-specific, then it might be relevant to have booleans instead, or possibly even both enum and booleans.

By the way, if we go for an enum, why not try to align it with what Android does (unless we find a standard we refer to that defines the values)

https://android.googlesource.com/platform/hardware/interfaces/+/master/automotive/vehicle/2.0/types.hal#3604

enum VehicleIgnitionState : int32_t {
    UNDEFINED = 0,
    LOCK = 1,
    OFF,
    ACC,
    ON,
    START
};

[gunnarx]

By the way, if we go for an enum, why not try to align it with what Android does (unless we find a standard we refer to that defines the values)

enum VehicleIgnitionState : int32_t {
UNDEFINED = 0,
LOCK = 1,
OFF,
ACC,
ON,
START
};

I agree. Consider my proposal to be aligned with this one instead.

[danielwilms]

Christian, will check internally and then we'll discuss.

[ChristianKerstan]

I've checked with some experts from our vehicle power net departments and we tried to align. However, we identified, that it might make sense to introduce additional enum values to cover EV specifics. This seems to be a big issue which is discussed in several automotive rounds... i.e. at 15th of October at VDA meeting.

I've invited the colleagues to our meeting at the 19th of October... can we please postpone until hearing them?

enum VehiclePowerState : int32_t {
	UNDEFINED	= 0,
	OFF, 			// parking (no key detected)
	LOCK, 			// steering locked (Lenksäulenverriegelung)
	ACC,			// accessories on / living
	ON,				// enable engine start (ignition on, diesel preheating, eDrive released)
	START 			// starting wish closes starter relay (not in EV)
	// Further OEM specific states might be added (e.g. charging, V2G, car wash….)
};

[danielwilms]

• we follow proposal (LowVoltageSystemState)
• we follow android order
• replace ingnitionOn to CombustionEngineRunning
• look at time signals in combination with ignitionOn
• include HV signals

[erikbosch]

Created a bunch of PRs according to the discussion on last meeting. Decided to put them in separate PRs as they are to a big extent independent.

[erikbosch]

@danielwilms @gunnarx - Now when #348 is merged I think this one can be closed

[danielwilms]

done in other PRs