Conversation
Codecov Report
@@ Coverage Diff @@
## main #8 +/- ##
============================================
- Coverage 97.95% 94.82% -3.14%
- Complexity 18 22 +4
============================================
Files 1 1
Lines 49 58 +9
============================================
+ Hits 48 55 +7
- Misses 1 3 +2
Continue to review full report at Codecov.
|
|
@atymic Thanks for updating! I'm not sure if
|
|
@chrisminett totally forgot about this. Will fix today :) |
|
@chrisminett ready for review now :) |
chrisminett
left a comment
chrisminett
left a comment
There was a problem hiding this comment.
Thanks for updating @atymic ! Not sure if I've spotted a potential issue. See comments on lines.
| // Any character outside the 7-bit alphabet switches the entire encoding to UCS-2 | ||
| $this->encoding = '7-bit'; | ||
| $this->size = 0; | ||
|
|
There was a problem hiding this comment.
No need to commit a change here.
| $this->messageCount = 1; | ||
| if ($this->size > $singleSize) { | ||
| $this->messageCount = (int)ceil($this->size / $concatSize); | ||
| $this->messageCount = (int) ceil($this->size / $concatSize); |
There was a problem hiding this comment.
No need to commit a change here.
| $this->size++; | ||
| } elseif (in_array($char, self::GSM0338_EXTENDED, true)) { | ||
| // In cases where a double counted char straddles two messages, add padding to push it to the next part | ||
| if (($this->size + 2) % self::MAXIMUM_CHARACTERS_7BIT_CONCATENATED === 1) { |
There was a problem hiding this comment.
I haven't verified this, but I'm concerned that if the extended character is encountered at pos 153 (so also using 154) in a message whose total length that is shorter than 160, this will be adding unnecessary size.
E.g. number of characters is 159, one extended char at pos 153 with size counted as 3, pushes total size to 161 where actually it would be 160 and fit in a single message.
There should be a test for this.
| if (in_array($char, self::GSM0338_BASIC, true)) { | ||
| $this->size++; | ||
| } elseif (in_array($char, self::GSM0338_EXTENDED, true)) { | ||
| // In cases where a double counted char straddles two messages, add padding to push it to the next part |
There was a problem hiding this comment.
I think it may be clearer to say why we need to count as 3; something like:
In cases where an extended char would straddle two messages, a padding control character is added first,
so the extended character is pushed entirely to the next part".
2 participants
No description provided.