Remove floating point from AT_CellularSMS #11396
Conversation
ciarmcom
requested review from
AnttiKauppila,
evedon,
hugueskamba and
a team
|
@gpsimenos, thank you for your changes. |
| @@ -1140,7 +1140,7 @@ bool AT_CellularSMS::create_time(const char *time_string, time_t *time) | |||
| &time_struct.tm_hour, &time_struct.tm_min, &time_struct.tm_sec, &sign, &gmt) == kNumberOfElements) { | |||
| *time = mktime(&time_struct); | |||
| // add timezone as seconds. gmt is in quarter of hours. | |||
| int x = 60 * 60 * gmt * 0.25; | |||
| int x = (60 / 4) * 60 * gmt; | |||
There was a problem hiding this comment.
It would make sense to have macros for these magic numbers too.
There was a problem hiding this comment.
Not convinced "60 seconds in a minute" is that magic. I don't like moving things off to macros elsewhere, unless it's a shared constant that's going to be reused.
If you want to spell it out, you could do so in the variable names themselves:
&tz_quarter_hours
int tz_minutes = tz_quarter_hours * 15;
int tz_seconds = tz_minutes * 60;
*time += tz_seconds;
(Using %c%d to grab the sign is also a bit weird - sscanf %d will handle + and - itself. Can't see a need to do it manually here).
|
CI started |
Test run: FAILEDSummary: 2 of 4 test jobs failed Failed test jobs:
|
|
CI started |
Test run: SUCCESSSummary: 11 of 11 test jobs passed |
Description
Replace unneccesary floating point operations in AT_CellularSMS with integers.
This is part of the requirements in IOTCLOUDPR-3289.
Pull request type
Reviewers
@evedon @hugueskamba @AnttiKauppila