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

Deprecating OBD-branch #701

Merged
merged 1 commit into from
Feb 13, 2024
Merged

Deprecating OBD-branch #701

merged 1 commit into from
Feb 13, 2024

Conversation

erikbosch
Copy link
Collaborator

@erikbosch erikbosch commented Jan 4, 2024

This is a follow up to #635. Intention is to have them deprecated and remove them first in VSS 6.0 Until then anyone can create PRs with replacement signals as needed. This PR contains replacement signals for diagnostics.

As a first point we should we discuss if this is a good approach for removing the OBD branch.

@erikbosch erikbosch marked this pull request as ready for review January 4, 2024 13:33
@ppb2020
Copy link
Collaborator

ppb2020 commented Jan 9, 2024

IIRC, our reasoning for deprecating OBD was that the signals were generally duplicates of other signals. I would have expected that some signals are not duplicated and would need to be added elsewhere in the catalog as part of the removal. I don't see other signals added...

I would also have expected that some ODB-specific signals might remain, for example a signal indicating that a device is connected to the OBD port. Or one where some "commands" can be sent in specifically to the OBD using certain pins on the diagnostic connector, which could be represented as signals specific to OBD... I don't know if OBD specifies this, however.

@erikbosch
Copy link
Collaborator Author

IIRC, our reasoning for deprecating OBD was that the signals were generally duplicates of other signals. I would have expected that some signals are not duplicated and would need to be added elsewhere in the catalog as part of the removal. I don't see other signals added...

In this PR I add signals for DTCCount and DTCList as we do not have DTCs elsewhere and several people have said that they could be useful. But you are correct, there are more signals that is not represented in VSS, especially in the area of combustion engine status (fuel trim, timing advance, lambda sensor readings and so on). But the question is if those signals are useful in VSS.

I can see multiple approaches here:

  • We could assume that everything is needed and create new signals for all that currently only exist in OBD. That could however be a quite tedious tasks - like how to represent lambda sensor banks for different engine configurations.
  • We could use this PR to discuss what signals we think are needed and only add replacement signals for those that someone request, as part of this PR
  • We could mark all as deprecated, and take the decision on which to create elsewhere later.

My original idea is to use the last approach, but allow long time for replacement signals to be created, i.e. remove OBD signals first in VSS 6.0. But your points are good, and we should align on the approach.

Copy link
Collaborator

@SebastianSchildt SebastianSchildt left a comment

Choose a reason for hiding this comment

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

I think this is good. Putting deprecation, but already migrating DTC (which is more or less the only point in OBD I have seen continuously being used"

I do agree with @ppb2020 that we should double check we are not "loosing" any potentially useful information from the OBD branch, but as deprecation does not imply immediate removall, we might want to do this in a seperate issue/PR?

@erikbosch
Copy link
Collaborator Author

Meeting notes:

  • Sebastian: We could possibly add OBD as an overlay, for people to grab. There may still be users that want to keep using OBD data. Possibly add overlay file already now.
  • AP: Erik to add info to changelog file. Add overlay, add to makefile

This is a follow up to COVESA#635
Intention is to have them deprecated and remove them first in VSS 6.0
Until then anyone can create PRs with replacement signals as needed.
This PR contains replacement signals for diagnostics.

Signed-off-by: Erik Jaegervall <[email protected]>
@erikbosch erikbosch added the Status:Meeting Intended to be discussed at next VSS-project meeting label Feb 13, 2024
@erikbosch
Copy link
Collaborator Author

MoM: OK to merge

@erikbosch erikbosch added Status:Approved Approved to merge and removed Status:Meeting Intended to be discussed at next VSS-project meeting labels Feb 13, 2024
@erikbosch erikbosch merged commit e511c1c into COVESA:master Feb 13, 2024
4 checks passed
@erikbosch erikbosch deleted the erik_obd branch February 13, 2024 16:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status:Approved Approved to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants