Skip to content

Conversation

@joeygrover
Copy link
Member

Fixes #1697

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

Core Tests

Tests performed:

  • Minimum protocol version check fail
  • Minimum RPC version check fail
  • SystemInfo check fail
  • Sending an UnregisterAppInterface and receiving a response
  • Receiving an OnAppInterfaceUnregistered notification. (Triggered via SDL_HMI ignition cycle)

Test code can be used to trigger an OnAppInterfaceUnregistered from the library. In the BaseLifecycleManager where RPCs are received, after the RAI processes the following code can be added to trigger a sending of the notification.

new Handler(Looper.getMainLooper()).postDelayed(new Runnable() {
    @Override
    public void run() {
        Log.d(TAG, "Sending OnAppInterfaceUnregistered. Icon may not clear from IVI, but service on phone should end. ");
        OnAppInterfaceUnregistered onAppInterfaceUnregistered = new OnAppInterfaceUnregistered();
        onAppInterfaceUnregistered.setReason(AppInterfaceUnregisteredReason.IGNITION_OFF);
        onReceived(onAppInterfaceUnregistered);
    }
}, 5000);

Summary

There exists a timing issue where the LCM can call for the lower layers to shutdown which will cause a flag to be set in the TransportManager that prevents the onTransportDisconnected callback from being triggered. Because of this, no layers above the TransportManager are notified. Simply adding the onClose call in the LCM after the calls to clean() the LCM will trigger the necessary callbacks.

Changelog

Bug Fixes
  • onDestroy of the SdlManagerListener will now be triggered in cases outside of transport disconnect.

CLA

@codecov
Copy link

codecov bot commented May 26, 2021

Codecov Report

Merging #1698 (3ba1159) into develop (889058f) will decrease coverage by 0.02%.
The diff coverage is 0.00%.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop    #1698      +/-   ##
=============================================
- Coverage      54.22%   54.20%   -0.03%     
+ Complexity      5351     5349       -2     
=============================================
  Files            555      555              
  Lines          24622    24628       +6     
  Branches        3122     3122              
=============================================
- Hits           13351    13349       -2     
- Misses         10111    10117       +6     
- Partials        1160     1162       +2     
Impacted Files Coverage Δ
...elink/managers/lifecycle/BaseLifecycleManager.java 14.07% <0.00%> (-0.13%) ⬇️
...managers/screen/TextAndGraphicUpdateOperation.java 70.60% <0.00%> (-0.55%) ⬇️

@joeljfischer joeljfischer added bug A defect in the library manager-lifecycle Relating to the manager layer - lifecycle manager labels May 27, 2021
@bilal-alsharifi bilal-alsharifi merged commit f0f1693 into develop Jun 2, 2021
@bilal-alsharifi bilal-alsharifi deleted the bugfix/issue_1697 branch June 2, 2021 17:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug A defect in the library manager-lifecycle Relating to the manager layer - lifecycle manager

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants