feat: Allow the specification of multiple charging ports. #755
Conversation
|
I saw some (deprectaed in 4.1) elements deleted, is my understanding correct , that this is with the assumption this chance would only go into a 5.x? But in that case maybe the "new deprecations" are not needed as well. OTOH, if we intend to add this to a 4.x release, I would assume leave everything in as deprecated? @erikbosch do you have an idea? Maybe we even documented our approach here? :D |
|
I deleted previous deprecations as they were referring to the same properties. Please, let me know whether such a change fits into a new 4.x release, or to a 5.0 release. I can make the corresponding modifications. |
|
MoM:
|
|
@erikbosch @SebastianSchildt , the PR was updated according to what was discussed yesterday. Could you please review it one more time? Thanks! |
Signed-off-by: Daniel Alvarez-Coello <8550265+jdacoello@users.noreply.github.com>
jdacoello
force-pushed
the
multiple-charging-ports
branch
from
786534b to
f36ef70
Compare
| Charging.ChargingPort: | ||
| type: branch | ||
| instances: | ||
| - ["FrontLeft", "FrontMiddle", "FrontRight", "RearLeft", "RearMiddle", "RearRight", "AnyPosition"] |
There was a problem hiding this comment.
The instances used here are somewhat different/reduced compared to previous set of allowed values. From my perspective not a problem but good to highlight when reviewing. The new values however correspond quite well with https://developer.android.com/reference/android/car/PortLocationType but there they use unspecified "front" instead of "FrontMiddle", and "Unknown" instead of "AnyPosition"
allowed: ['FRONT_LEFT', 'FRONT_MIDDLE', 'FRONT_RIGHT',
'REAR_LEFT', 'REAR_MIDDLE', 'REAR_RIGHT',
'LEFT_FRONT', 'LEFT_MIDDLE', 'LEFT_REAR',
'RIGHT_FRONT', 'RIGHT_MIDDLE', 'RIGHT_REAR']
description: Location of the charge port(s).
First part indicates side of vehicle, second part relative position on that side.
If supported, the list in this attribute shall have the same length as Charging.ChargePortType,
and use same the relative order.
comment: Example - If this signal is [LEFT_FRONT, RIGHT_FRONT] and Charging.ChargePortType is
[IEC_TYPE_2_AC, GBT_AC] that means that there is Mennekes port on the left side of the vehicle near
the front, and a GB/T AC port on the right side, near the front.
There was a problem hiding this comment.
I would say that the allowed_values are more normative in comparison to instances.
The instances are in the end arbitrary and often overwritten with overlays.
The main idea is to have an agreement on the branch that can be instantiated (e.g., Charging.ChargingPort) than in the particular occurrences (e.g., FRONT_LEFT).
I must admit that I am not a big fan of default instances as part of the main core specification. In my view, the core of the specification should be the general classes (e.g., Window, Door, ChargingPort) and their properties (position, isOpen, etc.). With that in place, one might further specify either more particular subclasses of them or instances.
@erikbosch, do you want me to update that list of instances in the PR?
There was a problem hiding this comment.
I do no really have any strong opinion, but why not base it on the terms in https://developer.android.com/reference/android/car/PortLocationType as default. (AnyPosition we could keep)
i.e.
- ["Front", "FrontLeft", "FrontRight", "Rear", "RearLeft", "RearRight", "AnyPosition"]
# OR
- ["Front", "FrontLeft", "FrontRight", "Rear", "RearLeft", "RearRight", "Unknown"]
|
MoM:
|
|
MoM: Merge. |
This PR addresses the issue described and discussed in #753 .