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

Sending UDS message (FF+CF) #278

Merged
merged 20 commits into from
Sep 23, 2024
Merged

Sending UDS message (FF+CF) #278

merged 20 commits into from
Sep 23, 2024

Conversation

mdabrowski1990
Copy link
Owner

Description

Sending of segmented UDS messages over CAN (using python-can package).

How Has This Been Tested?

  • unit tests
  • system tests

NOTE: Bug ticket #228 affects these changes.

Process

I know the process and did my best to follow it

Define where changes are required.
Add a few new system tests.
Prepare system tests
- Add creation of Flow Control packets (for the next task =/).
- Define Transmission Error and Waning.
- Core implementation of synchronous sending of FF+CF message.
- Implement async message sending
- add checks for n_br_measured
- rework n_cr_measured and n_bs_measured
- add timeout test after ff (detecting fc is received too late)
- Adjusted expectations in system tests
- Provided examples of timing issues with python-can
- unit tests for _send_packets_block
- unit tests for _async_send_packets_block
- unit tests for send_message (error handling - unexpected packet and transmission interrupted)
Provide all unit tests for sending message feature. Fix a few defects in code.
@mdabrowski1990 mdabrowski1990 added implementation Changes to code CAN Related to CAN specific implementation system tests New test cases for system or system integration testing labels Sep 21, 2024
@mdabrowski1990 mdabrowski1990 self-assigned this Sep 21, 2024
@mdabrowski1990 mdabrowski1990 linked an issue Sep 21, 2024 that may be closed by this pull request
Copy link

codecov bot commented Sep 21, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 100.00%. Comparing base (9469cad) to head (f23ef76).
Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff            @@
##              main      #278   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           39        39           
  Lines         2425      2503   +78     
  Branches       596       612   +16     
=========================================
+ Hits          2425      2503   +78     
Flag Coverage Δ
integration-tests 84.37% <28.48%> (-2.68%) ⬇️
integration-tests-branch 79.30% <27.21%> (-2.51%) ⬇️
unit-tests 100.00% <100.00%> (ø)
unit-tests-branch 100.00% <100.00%> (ø)
Files with missing lines Coverage Δ
uds/segmentation/can_segmenter.py 100.00% <100.00%> (ø)
uds/transport_interface/can_transport_interface.py 100.00% <100.00%> (ø)
uds/utilities/__init__.py 100.00% <100.00%> (ø)
uds/utilities/custom_exceptions.py 100.00% <100.00%> (ø)
uds/utilities/custom_warnings.py 100.00% <100.00%> (ø)

Add target to intergration tests branch coverage as 80% is used.
add flag so comments from bot contain coverage changes
must be set :(
Copy link
Owner Author

@mdabrowski1990 mdabrowski1990 left a comment

Choose a reason for hiding this comment

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

There are a few things to fix.

- update docstrings
- move and improve n_bs_measured and n_cr_measured to AbstractCanTransportInterface
- add more references to knowledge base
- improve async systemn tests (task handling)
- add missing assertions to unit tests
remove needless lines
- add system tests with message interrupting another message
- fix async_frames_bugger issue
Copy link
Owner Author

@mdabrowski1990 mdabrowski1990 left a comment

Choose a reason for hiding this comment

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

There is one serious defect.

Fix defect with asynchronous implementation where in some cases received packets were ignored during message transmission.
fixes before PR merging
Copy link
Owner Author

@mdabrowski1990 mdabrowski1990 left a comment

Choose a reason for hiding this comment

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

LGTM

@mdabrowski1990 mdabrowski1990 merged commit c4cd72d into main Sep 23, 2024
61 checks passed
@mdabrowski1990 mdabrowski1990 deleted the sending-uds-message-ff+cf branch September 23, 2024 17:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CAN Related to CAN specific implementation implementation Changes to code system tests New test cases for system or system integration testing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Sending UDS message (FF + CF)
1 participant