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.

Sign up for GitHub

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

Fixed queued menu updates failing #1958

Closed
wants to merge 5 commits into from
Closed

Fixed queued menu updates failing #1958

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f19462c
Merge branch 'develop' into bugfix/issue_1954_menu_updates_fail_on_sy…
NicoleYarroch Apr 6, 2021
6b3fe19
Fixed wrong queued menu getting sent
NicoleYarroch Apr 6, 2021
017b2ce
Added test cases for queued menu updates
NicoleYarroch Apr 7, 2021
a5ee8b0
Added newline
NicoleYarroch Apr 7, 2021
e34c41f
Cleaned up queued updates on stop
NicoleYarroch Apr 7, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 12 additions & 1 deletion SmartDeviceLink/private/SDLMenuManager.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,9 @@ @interface SDLMenuManager()
@property (assign, nonatomic) UInt32 lastMenuId;
@property (copy, nonatomic) NSArray<SDLMenuCell *> *oldMenuCells;

@property (copy, nonatomic) NSArray<SDLMenuCell *> *queuedDeleteMenuCells;
@property (copy, nonatomic) NSArray<SDLMenuCell *> *queuedAddMenuCells;

@end

UInt32 const ParentIdNotFound = UINT32_MAX;
Expand Down Expand Up @@ -120,6 +123,8 @@ - (void)stop {
_hasQueuedUpdate = NO;
_waitingOnHMIUpdate = NO;
_waitingUpdateMenuCells = @[];
_queuedAddMenuCells = @[];
_queuedDeleteMenuCells = @[];
}

#pragma mark - Setters
Expand Down Expand Up @@ -400,8 +405,11 @@ - (void)sdl_updateMenuWithCellsToDelete:(NSArray<SDLMenuCell *> *)deleteCells ce
if (self.inProgressUpdate != nil) {
// There's an in progress update, we need to put this on hold
self.hasQueuedUpdate = YES;
self.queuedAddMenuCells = addCells;
self.queuedDeleteMenuCells = deleteCells;
Comment on lines +408 to +409
Copy link
Contributor

Choose a reason for hiding this comment

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

Can this not use self.waitingUpdateMenuCells instead?

return;
}

__weak typeof(self) weakself = self;
[self sdl_sendDeleteCurrentMenu:deleteCells withCompletionHandler:^(NSError * _Nullable error) {
[weakself sdl_sendUpdatedMenu:addCells usingMenu:weakself.menuCells withCompletionHandler:^(NSError * _Nullable error) {
Expand All @@ -412,7 +420,10 @@ - (void)sdl_updateMenuWithCellsToDelete:(NSArray<SDLMenuCell *> *)deleteCells ce
}

if (weakself.hasQueuedUpdate) {
[weakself sdl_updateMenuWithCellsToDelete:deleteCells cellsToAdd:addCells completionHandler:nil];
SDLLogD(@"Sending queued menu updates");
[weakself sdl_updateMenuWithCellsToDelete:self.queuedDeleteMenuCells cellsToAdd:self.queuedAddMenuCells completionHandler:nil];
weakself.queuedDeleteMenuCells = @[];
weakself.queuedAddMenuCells = @[];
weakself.hasQueuedUpdate = NO;
}
}];
Expand Down
117 changes: 117 additions & 0 deletions SmartDeviceLinkTests/DevAPISpecs/SDLMenuManagerSpec.m
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@ @interface SDLMenuManager()
@property (assign, nonatomic) UInt32 lastMenuId;
@property (copy, nonatomic) NSArray<SDLMenuCell *> *oldMenuCells;

@property (copy, nonatomic) NSArray<SDLMenuCell *> *queuedDeleteMenuCells;
@property (copy, nonatomic) NSArray<SDLMenuCell *> *queuedAddMenuCells;

- (BOOL)sdl_shouldRPCsIncludeImages:(NSArray<SDLMenuCell *> *)cells;
- (void)sdl_displayCapabilityDidUpdate;

Expand Down Expand Up @@ -112,6 +115,8 @@ - (void)sdl_displayCapabilityDidUpdate;
expect(testManager.oldMenuCells).to(beEmpty());
expect(testManager.waitingUpdateMenuCells).to(beNil());
expect(testManager.menuConfiguration).toNot(beNil());
expect(testManager.queuedAddMenuCells).to(beNil());
expect(testManager.queuedDeleteMenuCells).to(beNil());
});

describe(@"updating menu cells before HMI is ready", ^{
Expand Down Expand Up @@ -621,6 +626,116 @@ - (void)sdl_displayCapabilityDidUpdate;
expect(adds).to(haveCount(6));
});
});

describe(@"updating the menu when the existing menu has not finished updloading", ^{
__block NSArray<SDLMenuCell *> *testCurrentMenuCells = nil;
__block NSArray<SDLMenuCell *> *testQueuedMenuCells = nil;

beforeEach(^{
testCurrentMenuCells = @[textOnlyCell, textAndImageCell];
testQueuedMenuCells = @[textOnlyCell, textOnlyCell2];
});

context(@"dynamic updates off", ^{
beforeEach(^{
testManager.dynamicMenuUpdatesMode = SDLDynamicMenuUpdatesModeForceOff;
OCMStub([mockFileManager uploadArtworks:[OCMArg any] completionHandler:[OCMArg invokeBlock]]);
testManager.menuCells = testCurrentMenuCells;
});

it(@"should save the menu cells that are queued and send them when the current menu finishes uploading", ^{
testManager.inProgressUpdate = @[];
testManager.menuCells = testQueuedMenuCells;

expect(testManager.hasQueuedUpdate).to(beTrue());
expect(testManager.queuedDeleteMenuCells).to(haveCount(2));
expect(testManager.queuedDeleteMenuCells).to(contain(textOnlyCell));
expect(testManager.queuedDeleteMenuCells).to(contain(textAndImageCell));

expect(testManager.queuedAddMenuCells).to(haveCount(2));
expect(testManager.queuedAddMenuCells).to(contain(textOnlyCell));
expect(testManager.queuedAddMenuCells).to(contain(textOnlyCell2));

expect(mockConnectionManager.receivedRequests).to(haveCount(2));
NSPredicate *deleteCommandPredicate = [NSPredicate predicateWithFormat:@"self isMemberOfClass:%@", [SDLDeleteCommand class]];
NSArray *deletes = [[mockConnectionManager.receivedRequests copy] filteredArrayUsingPredicate:deleteCommandPredicate];
NSPredicate *addCommandPredicate = [NSPredicate predicateWithFormat:@"self isMemberOfClass:%@", [SDLAddCommand class]];
NSArray *adds = [[mockConnectionManager.receivedRequests copy] filteredArrayUsingPredicate:addCommandPredicate];
expect(deletes).to(haveCount(0));
expect(adds).to(haveCount(2));

// When the current menu finishes uploading it should send the queued menu
[mockConnectionManager respondToLastMultipleRequestsWithSuccess:YES];
[mockConnectionManager respondToLastMultipleRequestsWithSuccess:YES];

expect(testManager.inProgressUpdate).to(beNil());
expect(testManager.hasQueuedUpdate).to(beFalse());
expect(testManager.queuedDeleteMenuCells).to(haveCount(0));
expect(testManager.queuedAddMenuCells).to(haveCount(0));

// Respond to the delete and add commands sent for the queued menu - (4 total)
[mockConnectionManager respondToLastMultipleRequestsWithSuccess:YES];
[mockConnectionManager respondToLastMultipleRequestsWithSuccess:YES];
[mockConnectionManager respondToLastMultipleRequestsWithSuccess:YES];
[mockConnectionManager respondToLastMultipleRequestsWithSuccess:YES];

NSArray *queuedMenuDeletes = [[mockConnectionManager.receivedRequests copy] filteredArrayUsingPredicate:deleteCommandPredicate];
NSArray *queuedMenuAdds = [[mockConnectionManager.receivedRequests copy] filteredArrayUsingPredicate:addCommandPredicate];
expect(queuedMenuDeletes).to(haveCount(2));
expect(queuedMenuAdds).to(haveCount(4));

expect(mockConnectionManager.receivedRequests).to(haveCount(6));
});
});

context(@"dynamic updates on", ^{
beforeEach(^{
testManager.dynamicMenuUpdatesMode = SDLDynamicMenuUpdatesModeForceOn;
OCMStub([mockFileManager uploadArtworks:[OCMArg any] completionHandler:[OCMArg invokeBlock]]);
testManager.menuCells = testCurrentMenuCells;
});

it(@"should save the menu cells that are queued and send them when the current menu finishes uploading", ^{
testManager.inProgressUpdate = @[];
testManager.menuCells = testQueuedMenuCells;

expect(testManager.hasQueuedUpdate).to(beTrue());
expect(testManager.queuedDeleteMenuCells).to(haveCount(1));
expect(testManager.queuedDeleteMenuCells).to(contain(textAndImageCell));

expect(testManager.queuedAddMenuCells).to(haveCount(1));
expect(testManager.queuedAddMenuCells).to(contain(textOnlyCell2));

expect(mockConnectionManager.receivedRequests).to(haveCount(2));
NSPredicate *deleteCommandPredicate = [NSPredicate predicateWithFormat:@"self isMemberOfClass:%@", [SDLDeleteCommand class]];
NSArray *deletes = [[mockConnectionManager.receivedRequests copy] filteredArrayUsingPredicate:deleteCommandPredicate];
NSPredicate *addCommandPredicate = [NSPredicate predicateWithFormat:@"self isMemberOfClass:%@", [SDLAddCommand class]];
NSArray *adds = [[mockConnectionManager.receivedRequests copy] filteredArrayUsingPredicate:addCommandPredicate];
expect(deletes).to(haveCount(0));
expect(adds).to(haveCount(2));

// When the current menu finishes uploading it should send the queued menu
[mockConnectionManager respondToLastMultipleRequestsWithSuccess:YES];
[mockConnectionManager respondToLastMultipleRequestsWithSuccess:YES];

expect(testManager.inProgressUpdate).to(beNil());
expect(testManager.hasQueuedUpdate).to(beFalse());
expect(testManager.queuedDeleteMenuCells).to(haveCount(0));
expect(testManager.queuedAddMenuCells).to(haveCount(0));

// Respond to the delete and add commands sent for the queued menu - (2 total)
[mockConnectionManager respondToLastMultipleRequestsWithSuccess:YES];
[mockConnectionManager respondToLastMultipleRequestsWithSuccess:YES];

NSArray *queuedMenuDeletes = [[mockConnectionManager.receivedRequests copy] filteredArrayUsingPredicate:deleteCommandPredicate];
NSArray *queuedMenuAdds = [[mockConnectionManager.receivedRequests copy] filteredArrayUsingPredicate:addCommandPredicate];
expect(queuedMenuDeletes).to(haveCount(1));
expect(queuedMenuAdds).to(haveCount(3));

expect(mockConnectionManager.receivedRequests).to(haveCount(4));
});
});
});
});

describe(@"running menu cell handlers", ^{
Expand Down Expand Up @@ -742,6 +857,8 @@ - (void)sdl_displayCapabilityDidUpdate;
expect(testManager.oldMenuCells).to(beEmpty());
expect(testManager.waitingUpdateMenuCells).to(beEmpty());
expect(testManager.menuConfiguration).toNot(beNil());
expect(testManager.queuedAddMenuCells).to(beEmpty());
expect(testManager.queuedDeleteMenuCells).to(beEmpty());
});
});

Expand Down