Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding additional signals to TractionBattery branch #601

Merged
merged 1 commit into from
Jun 13, 2023

Conversation

erikbosch
Copy link
Collaborator

This PR includes a few possible improvements to the TractionBattery branch.
I could give an introduction to the wanted changes in an upcoming VSS meeting

@erikbosch
Copy link
Collaborator Author

Meeting notes:

  • Please review
  • Nick: Is V2X is a good fit for Charging.Mode, is discharge, not charge. V2X may not be a good name. Possible GRID.Charge/GRID.Discharge. Should we add a value for "neither charge nor discharge"

@@ -290,12 +320,13 @@ Charging.ChargePlugType:
Charging.Mode:
datatype: string
type: actuator
allowed: ['MANUAL', 'TIMER', 'GRID', 'PROFILE']
allowed: ['MANUAL', 'TIMER', 'GRID', 'PROFILE', 'V2X']
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There were a short discussion on this signal last week - if V2X as name is misleading, if we need an IDLE state, if we need to split it into charge/discharge mode signals... That made me think - how is this signal intended to be used and does the current state reflect that. I am no expert in this area so feedback is appreciated. If we are to capture "modes" from a vehicle perspective, does the current "state space" reflects that? Do we possible need other states and a table, possibly like below.

I.e. That you in the vehicle (by settings) can control what shall happen if a charging cable is connected, and if the vehicle only will accept charge or also discharge? But do we then need to separate "GRID" from other charge/discharge modes? I.e. will the vehicle behave differently if you connect a "less intelligent load" like your boat or off-grid cottage or if you connect to a "smart Grid"? Is the difference possibly that in "manual/triggered discharge" we accept any load, but in grid-mode the "load/charger" must identify itself towards the vehicle with some trusted credentials.

Mode What happens when cable is connected? When will charging start? When will discharge start?
IDLE Nothing Never Never
MANUAL Charging starts Cable connected Never
TRIGGERED User dialog appears When acknowledged by user Never
TIMER Depends on timer Depends on timer Never
GRID Controlled by grid if authorized Controlled by grid if authorized Controlled by grid if authorized
PROFILE Depends on profile Depends on profile Never
MANUAL_CHARGE_DISCHARGE Charge or Discharge starts Cable connected Cable connected
TRIGGERED_CHARGE_DISCHARGE User dialog appears When acknowledged by user When acknowledged by user

One could off course also think off other combinations, like charging may start as soon as a charge is connected, but for discharge the driver needs to acknowledge it. Then having separate signals would be a benefit, potentially with different states.

If this discussion take too long time we could possibly handle it separately from the other changes.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: "MANUAL_CHARGE_DISCHARGE" is not clearly defined, so WHAT does happen when a cable is connected? Also generally not sure if "manual" is the correct term indicating a mode where connecting a cable AUTOMATICALLY does something

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: "MANUAL_CHARGE_DISCHARGE" is not clearly defined, so WHAT does happen when a cable is connected? Also generally not sure if "manual" is the correct term indicating a mode where connecting a cable AUTOMATICALLY does something

AUTOMATIC_CHARGE_DISCHARGE is maybe a better term. My assumption here is that the vehicle charge port here would act similar to a USB-port on a laptop. If you connect a charger the laptop will start charging. If you connect your phone (i.e. a load) the laptop will start charging your phone. I.e. the item you connect decide if it will charge or discharge the laptop. But I have no clue if that is how it will work also for a vehicle.

@erikbosch erikbosch force-pushed the erikbosch/erik_battery branch from 59a3bf4 to 5c5c0bf Compare May 22, 2023 12:14
GRID means grid-controlled (e.g. ISO 15118).
PROFILE means controlled by profile download to vehicle.
comment: The mechanism to provide a profile to the vehicle is currently not covered by VSS.
allowed: ['DEACTIVATED', 'AUTOMATIC', 'TRIGGERED', 'TIMER', 'PROFILE', 'V2X']
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Proposal refactored after internal discussion. For now we assume GRID can be considered as a special case for V2X, where you also have cases where you charge "stupid loads" by the vehicle, like charging your electrical chain saw. In both cases the vehicle is willing to provide energy to the load. It is assumed that the vehicle will have some internal limitations but apart from that it will try to give as much power as requested by the connected entity.

description: Identifier of the battery cell with highest voltage.
comment: Identifier is supposed to be relative index of the cell, starting with 0 the first cell.

CellVoltage.CellVoltage:
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems to cause a problem for ddsidl - we get a struct CellVoltage within a module CellVoltage which causes problem for idlc.

To me this seems like a bug in ddsidl - the VSS is correct, ddsidl needs to handle it. Off course we could avoid the problem by selecting a different name, but that is not the long term solution as I see it.

@SebastianSchildt
Copy link
Collaborator

Meeting 05/23

  • V2X maybe still not a good term (it is more associated with communication)
    • Need a different term maybe: EXTERNAL_ENTITY.
  • CellVoltage.CellVoltage: It does not seem "good style" to have a signal the same name as a branch, if it can be avoided
  • Should the signal maybe be CellVoltageS(plural, we did the some Error)

@erikbosch erikbosch force-pushed the erikbosch/erik_battery branch from 5c5c0bf to bac5bef Compare May 29, 2023 12:25
@erikbosch
Copy link
Collaborator Author

Meeting notes: PR updated, please review

@nickrbb
Copy link
Contributor

nickrbb commented Jun 6, 2023

In Charging.Mode, the values "MANUAL" and "GRID" are no longer allowed values as they were in v4.0 of this signal. Is their removal a backwards incompatible change?

If so, should we include them in 4.1 but state in the Deprecated field that these two values will be removed from version 5.0 and to use "TRIGGERED" and "EXTERNAL_ENTITY" instead?

@adobekan
Copy link
Collaborator

adobekan commented Jun 6, 2023

6.6.23 Decision: go for next week, check @nickrbb proposal.

Signed-off-by: Erik Jaegervall <erik.jaegervall@se.bosch.com>
@erikbosch erikbosch force-pushed the erikbosch/erik_battery branch from bac5bef to d2f42e3 Compare June 7, 2023 13:21
GRID means grid-controlled (e.g. ISO 15118).
PROFILE means controlled by profile download to vehicle.
comment: The mechanism to provide a profile to the vehicle is currently not covered by VSS.
deprecation: V4.1 - MANUAL and GRID are deprecated, please use AUTOMATIC/TRIGGERED or EXTERNAL_ENITY instead.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added MANUAL+GRID plus this deprecation text. Note that we here use the deprecation keyword in a somewhat new meaning as the signal itself is not deprecated but rather certain parts of it. We could alternatively just mention deprecation in free text

@erikbosch
Copy link
Collaborator Author

Meeting notes: Merge

@erikbosch erikbosch merged commit 7dbbb9e into COVESA:master Jun 13, 2023
@erikbosch erikbosch deleted the erikbosch/erik_battery branch June 19, 2023 11:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants