-
Notifications
You must be signed in to change notification settings - Fork 497
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
[VoiceBroadcast] Add the last sequence number in the paused/stopped state event #7136
Conversation
68dc34b
to
d41d03c
Compare
Codecov ReportBase: 11.76% // Head: 11.75% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## develop #7136 +/- ##
===========================================
- Coverage 11.76% 11.75% -0.02%
===========================================
Files 1620 1620
Lines 159388 159369 -19
Branches 64884 64878 -6
===========================================
- Hits 18759 18738 -21
- Misses 139978 139993 +15
+ Partials 651 638 -13
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
|
||
NSInteger lastChunkSequence = -1; | ||
if (JSONDictionary[VoiceBroadcastSettings.voiceBroadcastContentKeyChunkLastSequence]) { | ||
MXJSONModelSetInteger(chunkLength, JSONDictionary[VoiceBroadcastSettings.voiceBroadcastContentKeyChunkLastSequence]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MXJSONModelSetInteger(chunkLength, JSONDictionary[VoiceBroadcastSettings.voiceBroadcastContentKeyChunkLastSequence]); | |
MXJSONModelSetInteger(lastChunkSequence, JSONDictionary[VoiceBroadcastSettings.voiceBroadcastContentKeyChunkLastSequence]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in last commit.
@@ -66,8 +68,13 @@ + (id)modelFromJSON:(NSDictionary *)JSONDictionary | |||
voiceBroadcastId = relatesTo.eventId; | |||
} | |||
} | |||
|
|||
NSInteger lastChunkSequence = -1; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can use 0 as default value, because the actual first chunk sequence is 1
@yostyle can you confirm?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated in last commit. The first chunk sequence is 1 by default.
@@ -86,6 +93,10 @@ - (NSDictionary *)JSONDictionary | |||
JSONDictionary[VoiceBroadcastSettings.voiceBroadcastContentKeyChunkLength] = @(self.chunkLength); | |||
} | |||
|
|||
if (self.lastChunkSequence > 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if (self.lastChunkSequence > 0) { | |
if (self.lastChunkSequence != 0) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated in last commit.
@@ -241,10 +252,13 @@ extension VoiceBroadcastService { | |||
|
|||
/// Stop a voice broadcast. | |||
/// - Parameters: | |||
/// - lastChunkSequence: The last chunk number. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/// - lastChunkSequence: The last chunk number. | |
/// - lastChunkSequence: The last sent chunk number. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in last commit.
@@ -211,10 +219,13 @@ extension VoiceBroadcastService { | |||
|
|||
/// Pause a voice broadcast. | |||
/// - Parameters: | |||
/// - lastChunkSequence: The last chunk number. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/// - lastChunkSequence: The last chunk number. | |
/// - lastChunkSequence: The last sent chunk number. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in last commit.
if let lastChunkSequence = lastChunkSequence { | ||
voiceBroadcastInfo.lastChunkSequence = lastChunkSequence | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lastChunkSequence
is not initialized correctly here, we should add :
else voiceBroadcastInfo.lastChunkSequence = 0
Why don't we use the (initWithDeviceId...)[https://github.com//pull/7136/files#diff-3984ce62497db3e36cab4bcb87c1bb1d97d140510c5acba01b76c0b907d189caR22] method to init voiceBroadcastInfo
instance?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated in last commit.
Kudos, SonarCloud Quality Gate passed! |
Add the last sequence number in the paused/stopped state event for Voice Broadcast