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

Fix LCM to close down properly #1796

Merged
merged 6 commits into from
Mar 23, 2022
Merged

Fix LCM to close down properly #1796

merged 6 commits into from
Mar 23, 2022

Conversation

joeygrover
Copy link
Member

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 TDK and verified behavior (if applicable, if not applicable, explain why below).
  • I have tested Android, Java SE, and Java EE

Summary

This PR fixes some issues where the library is shutting down but the app wasn't unregistered from the IVI properly. It also fixes a SYNG 4 bug that caused incorrect app closing after a reregister.

Changelog

Bug Fixes
  • Fixed properly sending an UnregisterAppInterface when the LCM is shutting down
  • Removed redundant code that closed the RPC service before cleaning the whole LCM
  • Removed code from the UnregisterAppInterface response listener that caused issues with SYNC 4.
  • Added a missing init call in the Java SE LCM

CLA

This makes sure to send an UAI request when the LCM is shutting down to clear it from the head unit. Also removed redundent code to cose services and ignore the UAI response because there’s nothing the library can do at that point anyways.
@codecov
Copy link

codecov bot commented Mar 21, 2022

Codecov Report

Merging #1796 (5cde7bb) into 5.4.0_RC (7094f6f) will decrease coverage by 0.03%.
The diff coverage is 48.38%.

Impacted file tree graph

@@              Coverage Diff               @@
##             5.4.0_RC    #1796      +/-   ##
==============================================
- Coverage       54.09%   54.06%   -0.04%     
  Complexity       5519     5519              
==============================================
  Files             562      562              
  Lines           25714    25714              
  Branches         3372     3370       -2     
==============================================
- Hits            13909    13901       -8     
- Misses          10545    10556      +11     
+ Partials         1260     1257       -3     
Impacted Files Coverage Δ
...elink/managers/lifecycle/BaseLifecycleManager.java 15.29% <48.38%> (+1.52%) ⬆️
...com/smartdevicelink/protocol/SdlPacketFactory.java 51.42% <0.00%> (-11.43%) ⬇️
...rtdevicelink/streaming/video/SdlRemoteDisplay.java 50.00% <0.00%> (-2.44%) ⬇️
.../com/smartdevicelink/protocol/SdlProtocolBase.java 14.23% <0.00%> (-1.49%) ⬇️
...va/com/smartdevicelink/session/BaseSdlSession.java 15.65% <0.00%> (-1.02%) ⬇️
...com/smartdevicelink/util/MediaStreamingStatus.java 63.80% <0.00%> (-0.96%) ⬇️
...nk/managers/audio/AudioDecoderCompatOperation.java 79.54% <0.00%> (+4.54%) ⬆️

@JulianKast JulianKast changed the base branch from 5.4.0_RC to master March 22, 2022 20:35
@JulianKast JulianKast changed the base branch from master to 5.4.0_RC March 22, 2022 20:35
Copy link
Contributor

@JulianKast JulianKast left a comment

Choose a reason for hiding this comment

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

Just 3 very minor formatting changes are needed.

@JulianKast JulianKast merged commit 0f75391 into 5.4.0_RC Mar 23, 2022
@JulianKast JulianKast deleted the bugfix/unregister_fixes branch March 23, 2022 15:52
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.

2 participants