[PATCH 2/4]simutil: Changes to prepare for isimodem2.5

[PATCH 2/4]simutil: Changes to prepare for isimodem2.5

[Jessica Nilsson]

[-- Attachment #1: Type: text/plain, Size: 1154 bytes --]

updates in src/simutil.h
Adding some sim file id and sim dir id mostly related to phonebook usim/sim.

Regards,
Jessica Nilsson

---
 src/simutil.h |   21 +++++++++++++++++++++
 1 files changed, 21 insertions(+), 0 deletions(-)

diff --git a/src/simutil.h b/src/simutil.h
index 92b2e0f..bc53255 100644
--- a/src/simutil.h
+++ b/src/simutil.h
@@ -19,6 +19,8 @@
  *
  */
 
+#include "types.h"
+
 enum sim_fileid {
 	SIM_EFPL_FILEID = 0x2f05,
 	SIM_EF_ICCID_FILEID = 0x2fe2,
@@ -49,6 +51,25 @@ enum sim_fileid {
 	SIM_EFCBMIR_FILEID = 0x6f50,
 	SIM_EFCBMI_FILEID = 0x6f45,
 	SIM_EFCBMID_FILEID = 0x6f48,
+	SIM_EFSMSP_FILEID = 0x6f42,
+	SIM_EFIMSI_FILEID = 0x6F07,
+
+	/* Phonebook (USIM) */
+	SIM_EFPBR_FILEID = 0x4f30,
+	SIM_EFPSC_FILEID = 0x4F22,
+	SIM_EFCC_FILEID = 0x4F23,
+	SIM_EFPUID_FILEID = 0x4F24,
+	/* Phonebook (SIM) */
+	SIM_EFADN_SIM_FILEID = 0x6F3A,
+	SIM_EFEXT1_SIM_FILEID = 0x6F4A,
+};
+
+enum sim_dirid {
+	MF_FILEID = 0x3F00,
+	DFTELECOM_FILEID = 0x7F10,
+	DFGSM_FILEID = 0x7F20,
+	DFPHONEBOOK_FILEID = 0x5F3A,
+	DFMULTIMEDIA_FILEID = 0x5F3B
 };
 
 /* 51.011 Section 9.3 */
-- 
1.7.3.2

Re: [PATCH 2/4]simutil: Changes to prepare for isimodem2.5

[Marcel Holtmann]

[-- Attachment #1: Type: text/plain, Size: 2173 bytes --]

Hi Jessica,

> updates in src/simutil.h
> Adding some sim file id and sim dir id mostly related to phonebook usim/sim.

please write a proper commit message here. The subject is not acceptable
since it misleads what this patch is doing.

Also "updates in src/..." comments are not really useful. We have the
diffstat for this.

> ---
>  src/simutil.h |   21 +++++++++++++++++++++
>  1 files changed, 21 insertions(+), 0 deletions(-)
> 
> diff --git a/src/simutil.h b/src/simutil.h
> index 92b2e0f..bc53255 100644
> --- a/src/simutil.h
> +++ b/src/simutil.h
> @@ -19,6 +19,8 @@
>   *
>   */
>  
> +#include "types.h"
> +

What is this include for? Please remove it.

>  enum sim_fileid {
>  	SIM_EFPL_FILEID = 0x2f05,
>  	SIM_EF_ICCID_FILEID = 0x2fe2,
> @@ -49,6 +51,25 @@ enum sim_fileid {
>  	SIM_EFCBMIR_FILEID = 0x6f50,
>  	SIM_EFCBMI_FILEID = 0x6f45,
>  	SIM_EFCBMID_FILEID = 0x6f48,
> +	SIM_EFSMSP_FILEID = 0x6f42,
> +	SIM_EFIMSI_FILEID = 0x6F07,
> +
> +	/* Phonebook (USIM) */
> +	SIM_EFPBR_FILEID = 0x4f30,
> +	SIM_EFPSC_FILEID = 0x4F22,
> +	SIM_EFCC_FILEID = 0x4F23,
> +	SIM_EFPUID_FILEID = 0x4F24,
> +	/* Phonebook (SIM) */
> +	SIM_EFADN_SIM_FILEID = 0x6F3A,
> +	SIM_EFEXT1_SIM_FILEID = 0x6F4A,
> +};

So please be consistent with lower-case hex encoding. So 0x6f07 etc.

Also we should keep this information sorted. I know there is a sorting
bug in there that needs to be fixed as well.

And while at it, you could be the one that makes this enum finally
compliant to our coding style. This is one of our old left-overs that we
have get fixed. We just never got around to it. Are you up for such a
task?

So you could just fix the coding style in one patch, fix the sorting in
another and then add the new values.

> +enum sim_dirid {
> +	MF_FILEID = 0x3F00,
> +	DFTELECOM_FILEID = 0x7F10,
> +	DFGSM_FILEID = 0x7F20,
> +	DFPHONEBOOK_FILEID = 0x5F3A,
> +	DFMULTIMEDIA_FILEID = 0x5F3B
>  };

Please use a proper SIM_* prefix here and you can just put them into the
sim_fileid enum. However this should be a separate patch to get a clear
commit history of the changes.

Regards

Marcel

Re: [PATCH 2/4]simutil: Changes to prepare for isimodem2.5

[Jessica Nilsson]

[-- Attachment #1: Type: text/plain, Size: 991 bytes --]

Hi Marcel,
> please write a proper commit message here. The subject is not acceptable
> since it misleads what this patch is doing.
>    
>> +#include "types.h"
>> +
>>      
> What is this include for? Please remove it.
>    
> So please be consistent with lower-case hex encoding. So 0x6f07 etc.
  Ok, fix commit message, remove unnecessary include and use lower case. 
Will do.

> And while at it, you could be the one that makes this enum finally
> compliant to our coding style. This is one of our old left-overs that we
> have get fixed. We just never got around to it. Are you up for such a
> task?
>    
Sure, I'll take this. This is just the simutil.h we are talking about, 
not the simutil.c as well?

> So you could just fix the coding style in one patch, fix the sorting in
> another and then add the new values.
>    
3 patches to be sent in, and the last one to have the new values (with 
proper prefixes), I'll see to it.

Best Regards,
Jessica