See 3GPP TS 23.040 9.2.3.24.3. This issue is also the first stepping stone to WebMMS.

SMS IEI Application Port Addressing 8/16 bit allows short messages to be routed to one of multiple applications, using a method similar to TCP/UDP ports in a TCP/IP network. An application entity is uniquely identified by the pair of TP-DA/TP-OA and the port address. This issue should give a way to send[1]/receive octet data via SMS. I suggest new APIs for send: 1) dom/sms/interfaces/nsIDOMSmsManager: jsval sendData(in jsval number, in octet[] data); 2) dom/sms/interfaces/nsISmsService: void sendData(in DOMString number, in octet[] data, in long requestId, [optional] in unsigned long long processId); 3) dom/sms/interfaces/nsIRadioInterfaceLayer[2]: void sendSMSData(in DOMString number, in octet[] data, in long requestId, in unsigned long long processId); 4) embedding/android/SmsManager.java.in[3]: public void sendData(String aNumber, byte[] aData, int aRequestId, long aProcessId); PS1: Sending outgoing data via SMS is not required for WAP Push/MMS. PS2: Also need changes in dom/system/gonk/nsIRadioInterfaceLayer.js PS3: Also need changes in embedding/android/GeckoAppShell.java, mobile/android/base/GeckoAppShell.java, mobile/android/base/GeckoSmsManager.java, widget/android/AndroidBridge.cpp, widget/android/AndroidBridge.h For routing received data PDU, some application registration mechanism is mandatory. The original RadioInterfaceLayer uses kSmsReceivedObjserverTopic notification, but the created SmsMessage is text-based. Maybe we should another topic, say kSmsDataReceivedObjserverTopic. Any suggestions?

(In reply to Vicamo Yang [:vicamo] from comment #1) > This issue should give a way to send[1]/receive octet data via SMS. I > suggest new APIs for send: > > 1) dom/sms/interfaces/nsIDOMSmsManager: > > jsval sendData(in jsval number, in octet[] data); Should existing send/sendSMS/sendMessage methods be renamed to sendText/sendTextSMS/sendTextMessage as well?

sendText() would be ok for me. However, I would have prefer to pass Blob[] instead of octet[]. It seems way nicer to web authors. Is there a reason why you prefer octet[]? And I guess we would like to allow passing a SMSC to the sendData() method, right?

(In reply to Mounir Lamouri (:volkmar) (:mounir) from comment #3) > sendText() would be ok for me. I meant, there are 3 different names but related to the same function. It is `send` in nsIDOMMozSmsManager and nsISmsService, `sendSMS` in nsIRadioInterfaceLayer and `sendMessage` in android backend code. Should we rename all of them to the same one or keep these differences? I would vote for an unique `sendTextMessage`. > However, I would have prefer to pass Blob[] instead of octet[]. It seems way > nicer to web authors. Is there a reason why you prefer octet[]? Because I don't know there is some class named Blob ... I have to emphasize again that sending outgoing data via SMS is not required for WAP Push/MMS. This sentDataMessage API will probably be used as a utility function to implement/test port addressed data SMS receiving. That is, it's unlikely that any user will ever try to use this API. Besides, port addressed data SMS receiving is only for WAP Push PDUs, which are mostly for MMS availability indication. Another potential user would be network initiated AGPS[1], which is supported by Android. Again, it's sent via WAP Push, no outgoing data need. Both of them take raw bytes inside the SMS message. You may call them bytes/octets/blobs, anything that refers to a continuous space for data. And it will be "in Blob aBlob", not "in Blob[] aBlobs", right? Reference: 1) http://www.openmobilealliance.org/technical/release_program/supl_v1_0.aspx > And I guess we would like to allow passing a SMSC to the sendData() method, > right? That's in bug 736701, Reply Path. You can't reply a data based SMS message. Data based SMS messages are dispatched to registered applications and never get stored in SMS database. So we won't need originating SMSC/TP-RP/pid(for bug 736708) here.

(In reply to Vicamo Yang [:vicamo] from comment #4) > (In reply to Mounir Lamouri (:volkmar) (:mounir) from comment #3) > > And I guess we would like to allow passing a SMSC to the sendData() method, > > right? > > That's in bug 736701, Reply Path. You can't reply a data based SMS message. > Data based SMS messages are dispatched to registered applications and never > get stored in SMS database. So we won't need originating SMSC/TP-RP/pid(for > bug 736708) here. I'm wrong, Android API has it.

GAIA issue link is here: https://github.com/andreasgal/gaia/issues/921

See bug 740698 comment 5, we'll not implement sendData in this task thus it should no longer depends on bug 740698.

(In reply to Vicamo Yang [:vicamo] from comment #9) > Created attachment 613941 [details] [diff] [review] > Part 2: support 8-bit encoding This patch depends on bug 736941 attachment 610819 [details] [diff] [review] "Part 3: Add GsmPDUHelper.read/writeHexOctetArray"

Comment on attachment 613942 [details] [diff] [review] Part 3: parse application port address for incoming messages Review of attachment 613942 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/system/gonk/RadioInterfaceLayer.js @@ +421,5 @@ > > + // If application port addressing is available ... > + // Note that it can possibly be zero when representing a UDP/TCP port. > + if (typeof message.header.destinationPort != "undefined") { > + // TODO: dispatch to registered application Shall we have some kind of port registration in RadioInterfaceLayer? ::: dom/system/gonk/ril_worker.js @@ +2932,5 @@ > + header.originatorPort = orip; > + break; > + } > + case PDU_IEI_APPLICATION_PORT_ADDREESING_SCHEME_16BIT: { > + let dstp = (this.readHexOctet() << 8) | this.readHexOctet()); unpaired parenthesis

Comment on attachment 613941 [details] [diff] [review] Part 2: support 8-bit encoding Review of attachment 613941 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/system/gonk/ril_worker.js @@ +1730,5 @@ > delete this._receivedSmsSegmentsMap[hash]; > > // Rebuild full body > + if (options.encoding == PDU_DCS_MSG_CODING_8BITS_ALPHABET) { > + options.fullBody = Array.concat(options.segments); What are you trying to do here? This will simply make a copy of the array, not return a string. I'm pretty sure you want something else, but I don't know what. :)

Comment on attachment 613942 [details] [diff] [review] Part 3: parse application port address for incoming messages Review of attachment 613942 [details] [diff] [review]: ----------------------------------------------------------------- r+ with points addressed. ::: dom/system/gonk/RadioInterfaceLayer.js @@ +418,5 @@ > > handleSmsReceived: function handleSmsReceived(message) { > debug("handleSmsReceived: " + JSON.stringify(message)); > > + // If application port addressing is available ... Then what? @@ +420,5 @@ > debug("handleSmsReceived: " + JSON.stringify(message)); > > + // If application port addressing is available ... > + // Note that it can possibly be zero when representing a UDP/TCP port. > + if (typeof message.header.destinationPort != "undefined") { `if (message.header.destinationPort)`? Or can the port be zero? In that case, `if (message.header.destinationPort != null)` (we should accept both `undefined` and `null`, and undefined == null).

(In reply to Philipp von Weitershausen [:philikon] from comment #13) > Comment on attachment 613941 [details] [diff] [review] > Part 2: support 8-bit encoding > > Review of attachment 613941 [details] [diff] [review]: > ----------------------------------------------------------------- > > ::: dom/system/gonk/ril_worker.js > @@ +1730,5 @@ > > delete this._receivedSmsSegmentsMap[hash]; > > > > // Rebuild full body > > + if (options.encoding == PDU_DCS_MSG_CODING_8BITS_ALPHABET) { > > + options.fullBody = Array.concat(options.segments); > > What are you trying to do here? This will simply make a copy of the array, > not return a string. I'm pretty sure you want something else, but I don't > know what. :) Yes, I do want a concatenated array. In 8-bit encoding scheme applied and as a result of current text only SMS API, binary data will be processed in RadioInterfaceLayer.handleSmsReceived() and DISCARDED. We need binary data for WAP Push.

(In reply to Vicamo Yang [:vicamo] from comment #15) > > ::: dom/system/gonk/ril_worker.js > > @@ +1730,5 @@ > > > delete this._receivedSmsSegmentsMap[hash]; > > > > > > // Rebuild full body > > > + if (options.encoding == PDU_DCS_MSG_CODING_8BITS_ALPHABET) { > > > + options.fullBody = Array.concat(options.segments); > > > > What are you trying to do here? This will simply make a copy of the array, > > not return a string. I'm pretty sure you want something else, but I don't > > know what. :) > > Yes, I do want a concatenated array. What's a "concatenated array"? > In 8-bit encoding scheme applied and as > a result of current text only SMS API, binary data will be processed in > RadioInterfaceLayer.handleSmsReceived() and DISCARDED. We need binary data > for WAP Push. I see. How is binary data going to be represented?

(In reply to Philipp von Weitershausen [:philikon] from comment #16) > (In reply to Vicamo Yang [:vicamo] from comment #15) > > > ::: dom/system/gonk/ril_worker.js > > > @@ +1730,5 @@ > > > > delete this._receivedSmsSegmentsMap[hash]; > > > > > > > > // Rebuild full body > > > > + if (options.encoding == PDU_DCS_MSG_CODING_8BITS_ALPHABET) { > > > > + options.fullBody = Array.concat(options.segments); > > > > > > What are you trying to do here? This will simply make a copy of the array, > > > not return a string. I'm pretty sure you want something else, but I don't > > > know what. :) > > > > Yes, I do want a concatenated array. > > What's a "concatenated array"? I mean, the WSP(sessionless)/WDP/... delegate fragmentation handling to its bearer. They want a full PDU frame at each processing transaction. Therefore SMS segments of 8-bit encoding should be concatenated before submitting to upper layer. > > In 8-bit encoding scheme applied and as > > a result of current text only SMS API, binary data will be processed in > > RadioInterfaceLayer.handleSmsReceived() and DISCARDED. We need binary data > > for WAP Push. > > I see. How is binary data going to be represented? The binary data submitted to registered application will be an array of octets. For now we have only one such app, WAP Push, whose PDU parser will be implemented in bug 744360.

(In reply to Vicamo Yang [:vicamo] from comment #17) > (In reply to Philipp von Weitershausen [:philikon] from comment #16) > > (In reply to Vicamo Yang [:vicamo] from comment #15) > > > > ::: dom/system/gonk/ril_worker.js > > > > @@ +1730,5 @@ > > > > > delete this._receivedSmsSegmentsMap[hash]; > > > > > > > > > > // Rebuild full body > > > > > + if (options.encoding == PDU_DCS_MSG_CODING_8BITS_ALPHABET) { > > > > > + options.fullBody = Array.concat(options.segments); > > > > > > > > What are you trying to do here? This will simply make a copy of the array, > > > > not return a string. I'm pretty sure you want something else, but I don't > > > > know what. :) > > > > > > Yes, I do want a concatenated array. > > > > What's a "concatenated array"? > > I mean, the WSP(sessionless)/WDP/... delegate fragmentation handling to its > bearer. They want a full PDU frame at each processing transaction. Therefore > SMS segments of 8-bit encoding should be concatenated before submitting to > upper layer. Concatenated with what? Array.concat with just one argument doesn't make a whole lot of sense because it will just work like Array.slice(array). Array.concat takes several arrays and concatenates them together to make one big one. That doesn't seem to be what you want here, but I don't know what else you want. Also, options.fullBody is normally a string and I don't think we should change that; it would be very confusing. > > > In 8-bit encoding scheme applied and as > > > a result of current text only SMS API, binary data will be processed in > > > RadioInterfaceLayer.handleSmsReceived() and DISCARDED. We need binary data > > > for WAP Push. > > > > I see. How is binary data going to be represented? > > The binary data submitted to registered application will be an array of > octets. As in, a JS array of numbers? Or a JS string? Or a JS Typed Array?

(In reply to Philipp von Weitershausen [:philikon] from comment #18) > (In reply to Vicamo Yang [:vicamo] from comment #17) > Concatenated with what? Array.concat with just one argument doesn't make a > whole lot of sense because it will just work like Array.slice(array). > Array.concat takes several arrays and concatenates them together to make one > big one. That doesn't seem to be what you want here, but I don't know what > else you want. `concat creates a new array consisting of the elements in the this object on which it is called, followed in order by, for each argument, the elements of that argument (if the argument is an array) or the argument itself (if the argument is not an array).` ~ https://developer.mozilla.org/en/JavaScript/Reference/Global_Objects/Array/concat So `Array.concat(options.segments)` == `Array.concat(options.segments[0], options.segments[1], ...)`. Verified on FF. > Also, options.fullBody is normally a string and I don't think > we should change that; it would be very confusing. That's a good point. I can rename it. > As in, a JS array of numbers? Or a JS string? Or a JS Typed Array? Yup, a JS array of numbers.

(In reply to Vicamo Yang [:vicamo] from comment #19) > (In reply to Philipp von Weitershausen [:philikon] from comment #18) > > (In reply to Vicamo Yang [:vicamo] from comment #17) > > Concatenated with what? Array.concat with just one argument doesn't make a > > whole lot of sense because it will just work like Array.slice(array). > > Array.concat takes several arrays and concatenates them together to make one > > big one. That doesn't seem to be what you want here, but I don't know what > > else you want. > > `concat creates a new array consisting of the elements in the this object on > which it is called, Which is `options.segments` in your case. > followed in order by, for each argument, the elements of > that argument (if the argument is an array) or the argument itself (if the > argument is not an array).` ~ > https://developer.mozilla.org/en/JavaScript/Reference/Global_Objects/Array/concat You're not passing any extra arguments, so essentially you're just creating a copy of the options.segments array. > So `Array.concat(options.segments)` == `Array.concat(options.segments[0], > options.segments[1], ...)`. Verified on FF. You're misunderstanding the semantics of the first argument. The Array.* functions are shortcuts for Array.prototype.*.call, so the first argument is always the `this` of the operation. So it must be an array: Array.concat(a, b, c, ...) translates to Array.prototype.concat.call(a, b, c, ...) which is the long way of spelling a.concat(b, c, ...); If `a` is not an array, you're doing something very "interesting" that definitely doesn't make sense. > > Also, options.fullBody is normally a string and I don't think > > we should change that; it would be very confusing. > > That's a good point. I can rename it. > > > As in, a JS array of numbers? Or a JS string? Or a JS Typed Array? > > Yup, a JS array of numbers. Why not a typed array of the Uint8Array kind, if we're dealing with octets here anyway?

(In reply to Philipp von Weitershausen [:philikon] from comment #20) > (In reply to Vicamo Yang [:vicamo] from comment #19) > You're not passing any extra arguments, so essentially you're just creating > a copy of the options.segments array. Great! I should have JSON.stringified it before output. Ok, I'm wrong. Any suggestions? I find `Array.concat([], options.segments)` works for me, but I have no idea about its performance impact. > > Yup, a JS array of numbers. > > Why not a typed array of the Uint8Array kind, if we're dealing with octets > here anyway? That array is read from GsmPDUHelper.readHexOctetArray() being introduced in bug 736941 attachment 614307 [details] [diff] [review]. So I should modify the patch instead? I'll cancel review request for these patches for now and return when your comments are fully addressed.

(In reply to Vicamo Yang [:vicamo] from comment #21) > (In reply to Philipp von Weitershausen [:philikon] from comment #20) > > (In reply to Vicamo Yang [:vicamo] from comment #19) > > You're not passing any extra arguments, so essentially you're just creating > > a copy of the options.segments array. > > Great! I should have JSON.stringified it before output. Ok, I'm wrong. Any > suggestions? I find `Array.concat([], options.segments)` works for me, but I > have no idea about its performance impact. `[].concat(options.segments)` will just create a copy of `options.segments` as well, since it adds the elements of `options.segments` to an empty array. Really, I would just like to know what you're trying to do. Do you want to flatten an array of arrays or something? What are the elements of `options.segments`? > > > Yup, a JS array of numbers. > > > > Why not a typed array of the Uint8Array kind, if we're dealing with octets > > here anyway? > > That array is read from GsmPDUHelper.readHexOctetArray() being introduced in > bug 736941 attachment 614307 [details] [diff] [review]. So I should modify the patch instead? > > I'll cancel review request for these patches for now and return when your > comments are fully addressed. Yeah, I think having GsmPDUHelper.readHexOctetArray() return a typed array makes sense since it obviously returns octets and we know the size in advance.

(In reply to Philipp von Weitershausen [:philikon] from comment #22) > (In reply to Vicamo Yang [:vicamo] from comment #21) > > (In reply to Philipp von Weitershausen [:philikon] from comment #20) > Really, I would just like to know what you're trying to do. Do you want to > flatten an array of arrays or something? What are the elements of > `options.segments`? `options.segments` is an array of string/octets bodies of each SMS message segment. For 7bit/UCS2 encoding, each of its elements is a string; for 8bit, a array of octet numbers. > > That array is read from GsmPDUHelper.readHexOctetArray() being introduced in > > bug 736941 attachment 614307 [details] [diff] [review]. So I should modify the patch instead? > > > > I'll cancel review request for these patches for now and return when your > > comments are fully addressed. > > Yeah, I think having GsmPDUHelper.readHexOctetArray() return a typed array > makes sense since it obviously returns octets and we know the size in > advance. I just found the typed array has fewer API than a normal one. For example, there is no `concat` and `toString` for it. To concatenate full PDU payload back, one have to write several lines to create a new typed array with appropriate length and reassign all elements into the new array. The debug print takes also a hand-write for-loop as well. There isn't something like `toArray` API. If there isn't any way more conventient or any advantage that we can get from Uint8Array, I really want to avoid them.

1) assign 8-bit encoded user data to msg.data/fullData to prevent name confusion. See comment 18. 2) remove writeHexOctetArray reference for now. 3) fix array concatenation. See comment 13.

1) fix unpaired parenthesis. See comment 12 2) add application registration code. See comment 14

Comment on attachment 616591 [details] [diff] [review] Part 2: support 8-bit encoding : V2 Review of attachment 616591 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/system/gonk/ril_worker.js @@ +1859,5 @@ > > if (message.header && (message.header.segmentMaxSeq > 1)) { > message = this._processReceivedSmsSegment(message); > } else { > + if (options.encoding == PDU_DCS_MSG_CODING_8BITS_ALPHABET) { should be `message.encoding`

Fix comment 26.

Comment on attachment 616883 [details] [diff] [review] Part 2: support 8-bit encoding : V3 Nice!

Comment on attachment 616592 [details] [diff] [review] Part 3: parse application port address for incoming messages : V2 Review of attachment 616592 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/system/gonk/RadioInterfaceLayer.js @@ +419,4 @@ > handleSmsReceived: function handleSmsReceived(message) { > debug("handleSmsReceived: " + JSON.stringify(message)); > > + // If application port addressing is available ... If would be good if you could still complete that sentence or rephrase the comment somehow to be a coherent English sentence otherwise.

(In reply to Vicamo Yang [:vicamo] from comment #23) > (In reply to Philipp von Weitershausen [:philikon] from comment #22) > > (In reply to Vicamo Yang [:vicamo] from comment #21) > > > (In reply to Philipp von Weitershausen [:philikon] from comment #20) > > Really, I would just like to know what you're trying to do. Do you want to > > flatten an array of arrays or something? What are the elements of > > `options.segments`? > > `options.segments` is an array of string/octets bodies of each SMS message > segment. For 7bit/UCS2 encoding, each of its elements is a string; for 8bit, > a array of octet numbers. Got it. Thanks for explaining. > > > That array is read from GsmPDUHelper.readHexOctetArray() being introduced in > > > bug 736941 attachment 614307 [details] [diff] [review]. So I should modify the patch instead? > > > > > > I'll cancel review request for these patches for now and return when your > > > comments are fully addressed. > > > > Yeah, I think having GsmPDUHelper.readHexOctetArray() return a typed array > > makes sense since it obviously returns octets and we know the size in > > advance. > > I just found the typed array has fewer API than a normal one. For example, > there is no `concat` and `toString` for it. Yeah, Typed Arrays have the standard `toString` method which is not very helpful on any object. I also feel your pain with `concat`, I ran into the same thing in `Buf.growIncomingBuffer()`. That said, I think a Uint8Array is still the right choice for binary data (octets). > To concatenate full PDU payload > back, one have to write several lines to create a new typed array with > appropriate length and reassign all elements into the new array. We can create a helper. Also, perhaps we should just add a `concat` method to Typed Arrays, and perhaps even a useful `toString` method! Boot to Gecko is all about finding the pain points of the web and fixing them. :) > The debug print takes also a hand-write for-loop as well. You can use Array comprehensions: [x for each (x in typedarray)] It's not super short, but it's shorter than a loop ;)

Address to comment 29, and rebase to latest m-c.

https://hg.mozilla.org/integration/mozilla-inbound/rev/9e58a74d1634 https://hg.mozilla.org/integration/mozilla-inbound/rev/88b7eafda84e https://hg.mozilla.org/integration/mozilla-inbound/rev/2f422d4378a3

https://hg.mozilla.org/mozilla-central/rev/9e58a74d1634 https://hg.mozilla.org/mozilla-central/rev/88b7eafda84e https://hg.mozilla.org/mozilla-central/rev/2f422d4378a3