Mobile Phone API Review Comments

Mobile Phone API Review Comments

Reviewer / Art Barstow (Nokia Research Center)
Reviewer E-mail /
Company / Nokia
Document ID / CELF_MPP_CS_FR1_20060301
Date Submitted / 2006-04-21

Please use a separate comments document for each specification you review. You may add rows to the table as necessary. Indicate Y in the “Editorial?” column when the specific problem you are reporting is a simple editorial mistake, typo, etc.

Line Number / Problem / Proposed Solution / Editorial?
(Y/N)
Section 0. / This section must enurate/describe the architecture’s major architectural constraints. For example it appears this API only supports the WCDMA protocol (NB. Line 541). The document should also describe the general voice communication model that is implied in this specification e.g. section 1.2.1 implies a one call, two calls, AV call model, etc. call processing model.
Section 1.1.3 / Must define 1) TAF address; 2) Internal TAF; 3) External TAF. Add a normative reference to the TAF specification to section 0.1.1. Also shouldn’t real constants be defined for these concepts (e.g. lower bound and upper bound)?
Line 553 / The API should not prescribe a hardwired maximum constraint of three calls
Line 582 / Add VCS to the Preface document’s terminology section
Lines 589 and 594 / Enumerate the conditions these two different error codes will be used
Section2 1.2. – 1.2.20 / What exactly is the name of the enum for each of these sections? I presume they are unique but it certainly isn’t clear. / Define a different enum for each of these sections.
Line 587 / Must clarify what “(CELF_CS_FW_RESULT)” means / Probably should be removed from the section heading. Note this same error occurs in other sections e.g. 1.2.4, 1.2.13
Line 596 / Define BTYPE and describe the relationship between these values and the CS_LINE_TYPE. Also describe the semantics of CS_ANY and CS_NONE e.g. when are they used.
Line 608 / Define “Call reference”
Section 1.2.5 / Must define “CN_No” and state what it mean for it to be “valid” and “not valid”.
Section 1.2.6 / Describe what “_KIND_” means in this context.
Section 1.2.7 / Need to describe the use case for this enum
Line 630 / This sentence isn’t comprehensible (e.g. what “data”; what does “cause” mean in this context)
Line 640 / Need a semantic title for this section (i.e. replace NoCLI)
Line 647 / This sentence isn’t comprehensible
Line 654 / Clarify “Reservation” in this context
Section 1.2.10 and 1.2.11 / These two sections define identical enums. / Define different enums.
1.2.18 / Must clarify if the three “Network id info” constants are part of the RRC mode enum or if they are part of some different enum / Define a separate enum for the three network identification info constants
1.2.19 / Building a dependency on the PS API seems like a poor design choice. / Remove PS dependency.
1.2.21 / The use of all upper case letters for this enum is inconsistent with the Preface. / Change syntax to be consistent with the Preface. Also need to explain the use case for this enum.
Line 720 and many other places / The use of C++ comments “//” for the description is inconsistent throughout the docuemtn / Describe enums, constants, etc. consistently
Line 729 and several other places / The preface defines CelfMpEvent and not CELF_MP_EVENT / Change all occurrences of CELF_MP_EVENT to CelfMpEvent throughout the document
Sections 1.3.{1,2,3,4, …} / These sections define values for the category and subtype fields of the CelfMpEvent structure but the document does not specify a constant/enum for those values / Define the value space for the category and subtype fields.
Line 732 / The CelfMpCsComStatus enum is undefined / Define it. This also raises the question about whether a compiler will throw an error if this is an enum yet the “info” field is typed as an int.
Section 1.3 subsections / The datatypes defined in these sections use uppercase only and that is inconsistent with the API preface / Use consistent casing.
Section 1.3 subsections / The field names of several of the datatypes/structures use some upper-case letters / Use consistent casing
Section 1.3 / Some of these subsections define the info and subinfo field for the various events but some do not. / The value space for all of the info and subinfo fields should be defined for every event structure defined in this document.
Section 1.3.4 / The error codes and result codes should use enums rather than arbitrary (undefined) numbers.
Line 799 / CELF_CS_CME type is not defined
Line 806 / CELF_CS_NOTICE is not define
Line 817 / CELF_CS_CHAN_NOUSE is not defined
Section 1.3.9 / Describe each field in this structure
Line 856 / Should use the Line Status enum
Line 857 / Should use the coverage enum
Line 858 / Should use the RCC enum
Line 859 / Should use the network identification info enum
Line 861-865 / Should use the appropriate enum
Line 871 / CELF_SRVINFO_TITLE not defined
Line 872 / CELF_SRVINFO_DATA not defined
Line 881 / CELF_RESMSG_TITLE not defined
Line 882 / CELF_RESMSG_DATA not define
Section 1.4 / This section needs introductory material regarding these events e.g. their use case, the programming model, etc. It also needs to clarify the datatype of these various notification types.
Section 1.4.1 / This section needs to describe the use cases for these event types.
Section 1.4.2 / All of these types should includes MPP in their name i.e. CELF_MPP_CS_...
Section 1.4.4 / Must clarify the use cases for the various bit settings. Also why is there a dependency on the PS API?
Sections 2-63 / Remove the I/O field for the Return Values since that information is of no value and doing so would make this document consistent with the PS API document.
Section 2.2.1 and other sections / Line 954 incorrectly uses “ID” rather than “Id” / Fix all of these errors throughout the document
Line 955 / CelfMpCsNotifySet is undefined
Section 2.1.2 and others / This section and several others refer to CS Class events (e.g. CELF_MPP_CS_CLASS_*) but such events are not defined.
Line 1008, 1060 and others / The reference to sections 0.1.1 and 0.1 are incorrect.
Line 1017 / The structure should be “…Cs…
Section 4. / Need to define what “Communication Status” means in this context. Seems like one of the CS-specific enums should be returned. Line 1080’s reference to section 0.1 is not correct.
Line 1096 / CelfMpCallNo is undefined
Line 1097 / CelfMpConnectInfo is undefined
Line 1134-1136 / These sentences are incomprehensible.
Section 6 / CelfMpTime is not defined. This function must return CelfMpStatus since the app_id may be invalid.
Line 1169 / CelfMpCsOffHk undefined
Line 1198 / CELF_MP_STATUS_COM_TYPE_ERR is undefined
Section 8.1.5 / Should enumerate the events that are triggered as a side effect of calling this API.
Line 1283 / CelfMpCsDialBuffer is undefined
Line 1284 / CelfMpCsDialLen is undefined
Section 11.1.5 / Must clarify the use case for this function i.e. when should an application call this function and what are the pre-conditions (e.g. other functions that should be called before this function is called).
Section 12 / The use case for this function is unclear. Perhaps it should be replaced with a function that registers a callback and that callback function should be invoked when a call forward request is received. Also, why does this function not have an app_id argument?
Line 1463 / Text before “issued” is missing and hence the meaning of this sentence is unclear
Sections 13, 14, 15, 16, 17 / Must clarify the use cases for these functions since it is unclear when or why these functions should be called and why they are not handled via the API’s event callback mechanism. In some of these sections it appears the Functional Description is intended for an implementor of the API rather than for the application developer.
Line 1590 / CelfMpCsMop is undefined
Line 1591 / CelfMpCsCallRef is undefined / Perhaps this should be CelfMpCallRef
Section 17.1.3 / Missing AppId error return value
Line 1681 / CelfMpCsConReq is undefined
Section 18 / CelfMpChanNum is undefined. The return value list should include AppId Error.
Section 19 / 1) CelfMpDCFSet is undefined; 2) Return value should include AppId Error; 3) None of the constants listed for event class are defined; 4) based on the description of the event_set argument, this function should include a callback argument.
Section 20 / Same errors as Section 19 #2 and #3
Section 21 / CelfMpCsRecMsg is undefined
Sections 22 and 23 / Add AppID Error to the list of Return Values
Section 24 / 1) CelfMpUDComStatus is undefined; 2) None of the CS_UD_* return values are defined; 3) Should be an error in the return values e.g. if no 64K/AV communication has been started
Section 25 / 1) CelfMpAVComStatus is undefined; 2) None of the CS_UD_* return values are defined; 3) Should be an error in the return values e.g. if no internal/external AV communication has been started
Section 26 / 1) CelfMpRcvScene is undefined; 2) add AppID Error to the list of Return Values; 3) Type of Retrun Value in Line 2051 should be CelfMpCsComStatus; 4) Type at Line 2072 should also be CelfMpCsComStatus
Section 27 / 1) the constants listed for event_set argument are undefined; 2) CelfMpCsMtype is undefined; 3) Line 2171 references an incorrect Section
Section 28 / 1) the constants listed for event_set argument are undefined; 2) CelfMpCsMtype is undefined;
Section 29 / CelfMpReceptionLevel is undefined
Section 30 / Line 2268 contains an incorrect reference
Section 31 / CELF_CS_LINE_STATUS_EX and CelfMpCsCoverage are undefined
Sections 32 and 33 / CelfMpCsVMNum is undefined
Sections 34 and 35 / CelfMpCallSelect is undefined
Sections 36, 37 and others / CelfMpRegNum and CelfMpCsSrvData are undefined
Line 2431 and 2468 / Describe the semantics of the registration number
Section 37 / Need to clarify the memory management model for the data argument e.g. is memory malloc’ed during this call and if so is the application responsible for free’ing it.
Sections 37,38,39 / According to the Functional Descriptions, these functions are related to “supplementary” service information. The use of “supplemenatary” in this context needs clarification as well as the use case for these functions
Sections 33, 34, 36, 37, 38, 39, 40, 41, 42 / These functions make a distinction about “non-volatile memory”. Why is this distinction made in the API and not hidden from the application developer. Also, must define “supplemenatary response message information”.
Line 2554 / This description isn’t comprehensible
Section 40 / The Functional Description lacks a clear use case for this function. In particular it is unclear when an application should cal this function as it appears this function should include a callback as an argument.
Line 2577 / This sentence needs elaboration, in particular what is USSD (add it the Preface document’s terminology) and there should be an error if the data argument is not USSD.
Sections 44 and 45 / 1) CelfMpCsReconnectionTone is undefined 2) the RECONN_ON_T_* constants are not defined
Sections 46, 47 and 50 / CelfMpCsNoiseCancel is undefined
Sections 48 and 49 / 1. CelfMpCsQualAlarm is undefined; 2) the QUALITY_ALM_* constants are not defined
Line 2837 / What is the policy model for determining whether or not a noise canceller is permitted?
Section 51 / 1) CelfMpCsHiPrioCom is undefined; 2) the COMPRI_* constants are not defined
Sections 52, 53, 54 / CelfMpCsVmSound is not defind
Line 2888 / The API must not prescribe this behavior.
Section 55 / CelfMpCsAutoRcv is not defined
Sections 56, 57, 61 / CelfMpCsTimer is not defined
Section 58 / CelfMpCsDate is not defined
Sections 60 and 61 / Based on these function’s Functional Description, these function should be called celf_mp_cs_get_silent_time and celf_mp_cs_set_siltent_time, respectively.