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 new sensors to powertrain battery and fuel system. #536

Merged
merged 4 commits into from
Mar 16, 2023

Conversation

adobekan
Copy link
Collaborator

@adobekan adobekan commented Feb 10, 2023

Adding few more sensor descriptions for battery, fuel system and charging.

Signed-off-by: Adnan Bekan <adnan.bekan@bmwgroup.com>

@adobekan adobekan requested a review from erikbosch February 10, 2023 14:02
type: uint32
datatype: float
unit: s
description: The time displayed to the customer which is needed for the current charging process to reach complete charge.
Copy link
Collaborator

Choose a reason for hiding this comment

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

How will this one differ from Charging.TimeToComplete?

Is it remaining time or time stamp it is supposed to continue?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

TimeToComplete is related to ChargeLimit. And the one I have proposed is always related to full charge of current net capacity. I will add comment here.

datatype: float
type: sensor
unit: l
description: Net capacity of fuel tank in liters.
Copy link
Collaborator

Choose a reason for hiding this comment

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

What is the difference between "capacity" and "net capacity"? How come that it varies? I think a comment explaining difference would be good.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think i did here mistake with naming.

spec/Powertrain/Battery.vspec Outdated Show resolved Hide resolved
spec/Powertrain/Battery.vspec Outdated Show resolved Hide resolved
@@ -376,6 +383,13 @@ Charging.TimeToComplete:
E.g. if charging shall start in 3 hours and 2 hours of charging is needed,
then Charging.TimeToComplete shall report 5 hours.

Charging.Displayed:
Copy link
Collaborator

Choose a reason for hiding this comment

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

This seesm something between a "technical" value (i.e. when wen the concept is, that the raw data is in VSS and an UI may want to modify how to represent it to the user), and a pure HMI info (e.g. when we modfelled somehwere else whether speed is displayed in km/h or mph)

For example, I guess charging to 100% is sort of a corner case, where most users probably charge to 90% or whatever default a vehicle has. And as a user (if that is what "displayed" means, I am interested when my charging process finishes (i.e 50%; 19%, 100%, "until I am ready to go again", whatever I have chosen in UI).

Maybe I misunderstood what is meant with this datapoint, but then I would still argue either the name or the description is wrong

Copy link
Collaborator

Choose a reason for hiding this comment

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

Would not Charging.TimetoFull or something similar fit better Charging.Displayed does not say much what the signal contain

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have used same logic as we have for SoC. Current vs Displayed

Copy link
Collaborator

Choose a reason for hiding this comment

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

One can argue that the term StateOfCharge is a well known characteristic of battery, i.e. if you see a signal name like Vehicle.Powertrain.TractionBattery.StateOfCharge.Displayed then quite many people would be able to guess what unit/range the signal has. I am not as sure that Vehicle.Powertrain.TractionBattery.Charging.Displayed is as easy to understand.

But we maybe should take a step back and look at the scenarios. We have Charging.ChargeLimit which indicates max-limit, like if we reach it charging shall stop automatically, and Charging.TimeToComplete is the time to that limit. In the meeting yesterday we rather talked about charging that shall continue after reaching the target, and two times/durations are needed, one for remaining time until target and one for remaining time until full. Now I assume that it is the "real" value we want to show - the reason for having two signals for StateOfCharge (StateOfCharge.Displayed and StateOfCharge.Current) is that by some reasons OEMs does not want to show the real value.

If we assume that we need to support two scenarios:

  • The user/driver can specify a minimum limit. When charging has reached that limit the user will be informed in his app, but charging will continue
  • The user/driver can specify a maximum limit (or it is decided by vehicle, possibly fixed at 100%). When charging has reached that limit the user will be informed in his app, and charging will stop.

Then we maybe need four signals (names can be discussed):

  • Charging.ChargeLimit to be replaced with Charging.MinimumChargeLimit and Charging.MaximumChargeLimit
  • Charging.TimeToComplete to be replaced by Charging.TimeToMinimumChargeLimit and Charging.TimeToMaximumChargeLimit

But which time that will actually will be shown in the vehicle or in the app may vary, I assume. If you just want enough charge to be able to drive home then I assume Charging.TimeToMinimumChargeLimit is what you are most interested in.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think i will change setup and use existing time information with modification on ChargeLimit with default value to 100. Then it should cover use-cases.
@erikbosch but also your proposal is good with adding more signals. Maybe I can add those as well, I would not add TimeToMinimum as an actuator. It is hard to say what is the minimum for driver. Or you have some scenario in mind?

ChargeLimit is what customer defines as an input, actuator. e.g. 70%
TimeToMinimumChargeLimit I see as a value that could be proposed to customer, sensor.

Copy link
Collaborator

@ppb2020 ppb2020 Mar 14, 2023

Choose a reason for hiding this comment

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

Regarding:

The user/driver can specify a minimum limit. When charging has reached that limit the user will be informed in his app, but charging will continue

Would it make sense for the app to inform the user based on its own monitoring of the charge level, as opposed to pushing the decision and monitoring into the vehicle? What if there are multiple applications each with their own minimum and maximum charge limits for the same vehicle (two partners sharing a vehicle)?

If a limit is set using Charging.MinimumChargeLimit, what is the signal that is triggered by reaching this limit? As well, from what direction is this limit monitored? Charging (as implied by the name) or discharging (where the user may want to be informed when the charge level reaches this limit while discharging)?

@erikbosch
Copy link
Collaborator

Meeting notes: Please review/comment. Possible merge decision Feb 21st.

@erikbosch
Copy link
Collaborator

Meeting notes: Adnan to update PR today, all review, merge decision next meeting

@erikbosch
Copy link
Collaborator

Meeting notes: merge

@erikbosch erikbosch merged commit 1020b7c into COVESA:master Mar 16, 2023
jdacoello pushed a commit to jdacoello/vehicle_signal_specification that referenced this pull request May 2, 2023
* Adding new sensors to powertrain battery and fuel system.

* fix errors

* update names

* update PR, according to discussion
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