From d1cc6bfee73b7277ad93cfcda13006f80e98ab55 Mon Sep 17 00:00:00 2001 From: Kevin Tang Date: Tue, 16 Sep 2014 22:43:56 -0700 Subject: [PATCH] GPS LOCK did not work when NMEA_PROVIDER follows it there is an implicit requirement on the loc_gps_cfg_s_type field data type, that is they must be 32 bit fields. Otherwise it would only work with the assistance of padding. When two adjacent 8 bit fields are defined, the later filled field would overwrite the previously written neighbor. This is why GPS_LOCK was tested as broken in the latest build. This also fixes a theoretic bug that when there are two of the same fields defined in the config table to be filled, the accounting of the filled entries was incorrect earlier. This is not a realistic bug, as there are no idential entries in the config table HAL fills. Bug: 16131208 CRs-fixed: 736966 Change-Id: I2e262fb30272f6f334508df17bb640022d7b1ef5 --- loc_api/libloc_api_50001/loc_eng.h | 12 +++++++++++- utils/loc_cfg.cpp | 5 ++--- 2 files changed, 13 insertions(+), 4 deletions(-) diff --git a/loc_api/libloc_api_50001/loc_eng.h b/loc_api/libloc_api_50001/loc_eng.h index 86bdae43..da185462 100644 --- a/loc_api/libloc_api_50001/loc_eng.h +++ b/loc_api/libloc_api_50001/loc_eng.h @@ -140,6 +140,9 @@ typedef struct loc_eng_data_s } loc_eng_data_s_type; /* GPS.conf support */ +/* NOTE: the implementaiton of the parser casts number + fields to 32 bit. To ensure all 'n' fields working, + they must all be 32 bit fields. */ typedef struct loc_gps_cfg_s { uint32_t INTERMEDIATE_POS; @@ -147,17 +150,24 @@ typedef struct loc_gps_cfg_s uint32_t SUPL_VER; uint32_t CAPABILITIES; uint32_t LPP_PROFILE; - uint8_t NMEA_PROVIDER; uint32_t XTRA_VERSION_CHECK; char XTRA_SERVER_1[MAX_XTRA_SERVER_URL_LENGTH]; char XTRA_SERVER_2[MAX_XTRA_SERVER_URL_LENGTH]; char XTRA_SERVER_3[MAX_XTRA_SERVER_URL_LENGTH]; uint32_t USE_EMERGENCY_PDN_FOR_EMERGENCY_SUPL; + uint32_t NMEA_PROVIDER; uint8_t GPS_LOCK; uint32_t A_GLONASS_POS_PROTOCOL_SELECT; uint32_t AGPS_CERT_WRITABLE_MASK; } loc_gps_cfg_s_type; +/* NOTE: the implementaiton of the parser casts number + fields to 32 bit. To ensure all 'n' fields working, + they must all be 32 bit fields. */ +/* Meanwhile, *_valid fields are 8 bit fields, and 'f' + fields are double. Rigid as they are, it is the + the status quo, until the parsing mechanism is + change, that is. */ typedef struct { uint8_t GYRO_BIAS_RANDOM_WALK_VALID; diff --git a/utils/loc_cfg.cpp b/utils/loc_cfg.cpp index b4b3ded4..404f3ed2 100644 --- a/utils/loc_cfg.cpp +++ b/utils/loc_cfg.cpp @@ -175,8 +175,7 @@ DEPENDENCIES N/A RETURN VALUE - 0: No config or incomplete config or invalid parameter - 1: Filled a record + 0: Number of records in the config_table filled with input_buf SIDE EFFECTS N/A @@ -220,7 +219,7 @@ int loc_fill_conf_item(char* input_buf, for(uint32_t i = 0; NULL != config_table && i < table_length; i++) { if(!loc_set_config_entry(&config_table[i], &config_value)) { - ret = 1; + ret += 1; } } }