Skip to content

Conversation

@joeygrover
Copy link
Member

Fixes #1678

This PR is ready for review.

Risk

This PR makes no API changes.

Testing Plan

  • I have verified that I have not introduced new warnings in this PR (or explain why below)
  • I have run the unit tests with this PR
  • I have tested this PR against an IVI system
  • I have tested Android, Java SE, and Java EE

Unit Tests

Additional tests were added to test the SdlPsm.java class and some tests were refactored. These included more corner case issues as provided in #1678.

Core Tests

Basic smoke testing can be done to ensure the packet state machine still works as intended. It is difficult to perform tests that would have actually broken the PSM so we rely on the unit tests for those scenarios.

Summary

  • Surrounded the base operation of the SdlPsm class in a try/catch that will set the state to ERROR state and safely exit. This prevents exceptions from being thrown in developer's applications. A log is written out through the DebugTool
  • Added a check for correct service types. This helps with recovery tremendously.
  • Added better dataSize checking per early versions of the protocol. Dynamic MTUs are not included, but adding them might not actually help that much since they could be extremely large anyways.
  • Refactored some checks to reduce redundant logic.

Changelog

Enhancements
  • Added better checks into the SdlPsm class
Bug Fixes
  • Added safeguards to prevent crashes from propagating to developer's apps.

CLA

joeygrover added 2 commits May 7, 2021 13:57
Add more checks that will help the PSM churn through bad data to get back on track. Also add a try/catch block for each attempt to prevent any production crashes.
@codecov
Copy link

codecov bot commented May 7, 2021

Codecov Report

Merging #1683 (676b546) into develop (a0f080a) will increase coverage by 0.24%.
The diff coverage is 55.55%.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop    #1683      +/-   ##
=============================================
+ Coverage      53.94%   54.19%   +0.24%     
- Complexity      5307     5343      +36     
=============================================
  Files            555      555              
  Lines          24562    24627      +65     
  Branches        3101     3115      +14     
=============================================
+ Hits           13251    13347      +96     
+ Misses         10160    10114      -46     
- Partials        1151     1166      +15     
Impacted Files Coverage Δ Complexity Δ
...ain/java/com/smartdevicelink/transport/SdlPsm.java 66.99% <52.00%> (+46.33%) 30.00 <2.00> (+25.00)
...om/smartdevicelink/streaming/StreamPacketizer.java 56.89% <100.00%> (ø) 12.00 <0.00> (ø)
...tdevicelink/streaming/video/RTPH264Packetizer.java 71.00% <100.00%> (ø) 28.00 <0.00> (ø)
...ink/managers/screen/BaseTextAndGraphicManager.java 63.75% <0.00%> (-0.42%) 50.00% <0.00%> (-2.00%)
.../main/java/com/smartdevicelink/util/DebugTool.java 17.20% <0.00%> (+2.15%) 12.00% <0.00%> (+1.00%)
.../managers/screen/menu/BaseVoiceCommandManager.java 68.84% <0.00%> (+6.93%) 23.00% <0.00%> (+12.00%)

Co-authored-by: Julian Kast <Julian.kast@livio.io>
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.

3 participants