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

VCU Simulation Framework #60

Merged
merged 78 commits into from
Jan 31, 2024
Merged

VCU Simulation Framework #60

merged 78 commits into from
Jan 31, 2024

Conversation

kevinl03
Copy link
Contributor

@kevinl03 kevinl03 commented Nov 3, 2023

Overview

Develop a comprehensive VCU Simulation Framework that empowers users to conduct detailed testing of the vehicle's firmware under race-like scenarios. This framework will facilitate thorough verification of the VCU's behavior in response to various critical inputs and stimuli, including

APPS (Accelerator Pedal Position Sensor)
BSE (Brake System Enable) pedals
RTD (Ready-To-Drive) signals
Set-Reset-Flip Events
TSAL (Transmission Shift Actuator Lock)

Development discussion

A detailed discussion about the features and scope of the framework can be found via this github issue #61

User Manual

Refer to the following document for environment set up and framework usage
https://docs.google.com/document/d/1h_xk5TsREIRYUv_hB6RO2n2hTzEec8QP7Y_7VT4gCtU/edit

Design Document

The following document outlines the framework design decisions for the VCU testing robustness. Developers who wish to modify the current usage should refer to

https://docs.google.com/document/d/10IChPywZE57cQT93kg4YBlf_Gsr4lBgPxA6Phs91sME/edit

High level architecture

image

Framework implementation:

VCU Test Framework UML

kevinl03 and others added 30 commits July 22, 2023 21:22
…cess will be distributed privately to avoid uploading to our public Team Phantom repo
…t gmail editor access using google drive API v3
@kevinl03 kevinl03 added this to the Phantom Release 2 milestone Nov 7, 2023
Copy link
Contributor

@kkramer215 kkramer215 left a comment

Choose a reason for hiding this comment

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

Comments for the functions and classes are very helpful while following along with the code. Just make a note for each of the comments in the files and resolve any merge conflicts but looks really good!

Copy link
Collaborator

@rafguevara14 rafguevara14 left a comment

Choose a reason for hiding this comment

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

Consider that with regard to readability and maintainability, the intention of your code should be front and center, and be as obvious as possible. In some places, the use of type annotations dominates the code more than they would in a strongly typed language like C++.

Long type annotations:

These types with nested dicts and lists should either be declared as its own class or type aliased. The information is still relevant, but we should only have to read the super-long definition when we need to.

# you have to get past the type annotation to see the actual intention of the code
write_res : dict[Union[VCU_Pedal, str], Union[list[int], list[EventData], list[StateData]]] = self._write_data()
-------------------
# at top of file
VCUResult = dict[Union[VCU_Pedal, str], Union[list[int], list[EventData], list[StateData]]] 

write_res : VCUResult = self._write_data()

Redundant annotations:
Adding type annotations for every single line of code can add unnecessary noise to the codebase; not all situations need strict typing. This is a little more subjective about where you choose to forego type annotation.

A good rule to follow is to type annotate function arguments and return values. Types can then be deduced from these return values. If it's not obvious, then add a type annotation.

json_config: dict = self._parse_json()
def _parse_json(self) -> Union[dict, None]:

-------
# the IDE and the programmer can deduce the type easily from this simple assignment. 
json_config = self._parse_json() 

Incorrect, misleading, and/or non-obvious:

interface: VCUSimInterface = VCUSimInterface().begin() # the begin method does not actually return a VCUSimInterface

Also found a stack overflow post that I 100% agree with giving more advice on type hints
image

Copy link
Collaborator

@rafguevara14 rafguevara14 left a comment

Choose a reason for hiding this comment

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

This should be a good baseline for the project. Now we can move this over to its own repo and continue development there

@kkramer215 kkramer215 merged commit 63aa28f into develop Jan 31, 2024
kevinl03 added a commit that referenced this pull request Feb 4, 2024
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.

Create VCU Simulation Framework for Integration Testing
3 participants