Implement GetMessagesListing function

This patch depends on bug1180554 patch (iplh) file.

Comment on attachment 8648722 [details] [diff] [review] Bug 1166675 - Implement GetMessagesListing function Review of attachment 8648722 [details] [diff] [review]: ----------------------------------------------------------------- r=me with comment addressed. ::: dom/bluetooth/BluetoothCommon.h @@ +230,5 @@ > */ > #define REQUEST_MEDIA_PLAYSTATUS_ID "requestmediaplaystatus" > > /** > + * When receiving a MAP request from a remote device, we'll dispatch an a MAP request 'of messages listing' from a remote device ... ::: dom/bluetooth/bluedroid/BluetoothMapSmsManager.cpp @@ +699,5 @@ > + /* > + * Follow MAP 6.3.1 Application Parameter Header > + */ > + switch (aTagId) { > + case Map::AppParametersTagId::MaxListCount: { We should make |BT_APPEND_NAME_VALUE| a function to take parameter and remove redundancy here. Please open a follow-up bug for it. @@ +700,5 @@ > + * Follow MAP 6.3.1 Application Parameter Header > + */ > + switch (aTagId) { > + case Map::AppParametersTagId::MaxListCount: { > + if (!aHeader.GetAppParameter(Map::AppParametersTagId::MaxListCount, Check at the beginning of the function. uint8_t buf[64]; if (!aHeader.GetAppParameter(aTagId, buf, 64)) { return; } @@ +843,5 @@ > + nsString name; > + aHeader.GetName(name); > + BT_APPEND_NAMED_VALUE(data, "name", name); > + > + AppendBtNamedValueByTagId(aHeader, data, I prefer to have a array for these app parameters: static Map::AppParametersTagId sMsgListingParameters = { CONVERT(0, Map::AppParametersTagId::MaxListCount); ... CONVERT(n, Map::AppParametersTagId::FilterPriority); } for (uint8_t i = 0; i < MOZ_ARRAY_LENGTH(sMsgListingParameters); i++) { AppendBtNamedValueByTagId(aHeader, data, sMsgListingParameters[i]); } @@ +867,5 @@ > + AppendBtNamedValueByTagId(aHeader, data, > + Map::AppParametersTagId::FilterPriority); > + > + #ifdef MOZ_B2G_BT_API_V1 > + bs->DistributeSignal( Remove v1 implementation since it's already removed on m-c. Also restore the indention.

(In reply to Ben Tian [:btian] from comment #8) > Comment on attachment 8648722 [details] [diff] [review] > Bug 1166675 - Implement GetMessagesListing function > > Review of attachment 8648722 [details] [diff] [review]: > ----------------------------------------------------------------- > > r=me with comment addressed. > > ::: dom/bluetooth/BluetoothCommon.h > @@ +230,5 @@ > > */ > > #define REQUEST_MEDIA_PLAYSTATUS_ID "requestmediaplaystatus" > > > > /** > > + * When receiving a MAP request from a remote device, we'll dispatch an > > a MAP request 'of messages listing' from a remote device ... > > ::: dom/bluetooth/bluedroid/BluetoothMapSmsManager.cpp > @@ +699,5 @@ > > + /* > > + * Follow MAP 6.3.1 Application Parameter Header > > + */ > > + switch (aTagId) { > > + case Map::AppParametersTagId::MaxListCount: { > > We should make |BT_APPEND_NAME_VALUE| a function to take parameter and > remove redundancy here. Please open a follow-up bug for it. > I'm not sure I understand your point. Are you saying BT_APPEND_NAME_VALUE shall take AppParametersTagId as parameter?

(In reply to Shawn Huang [:shawnjohnjr] from comment #9) > > We should make |BT_APPEND_NAME_VALUE| a function to take parameter and > > remove redundancy here. Please open a follow-up bug for it. > > > I'm not sure I understand your point. Are you saying BT_APPEND_NAME_VALUE > shall take AppParametersTagId as parameter? Yes. But it can't because of being macro instead of function, per bug 1180554 comment 6. "I had tried to revise the codes by using a literal strings array, but I can't find a proper way to do this. One of the reasons is that the array will be used as the second parameter of 'BT_APPEND_NAMED_VALUE'. It means the array will be manipulated by the macro 'NS_LITERAL_STRING' first before the compilation process."

(In reply to Ben Tian [:btian] from comment #10) > Yes. But it can't because of being macro instead of function, per bug > 1180554 comment 6. > > "I had tried to revise the codes by using a literal strings array, but I > can't find a proper way to do this. > > One of the reasons is that the array will be used as the second parameter of > 'BT_APPEND_NAMED_VALUE'. It means the array will be manipulated by the macro > 'NS_LITERAL_STRING' first before the compilation process." I opened bug 1195685 to track it.

Sorry, Ben. I found I made a mistake :(. MessageListing function does not contain 'name', I will remove it.

Rebase to the latest m-c.

Re-upload the file, the previous upload is old version.

try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=0b670b338214

Comment on attachment 8659371 [details] [diff] [review] Bug 1166675 - Implement GetMessagesListing function, r=btian (m-c) Review of attachment 8659371 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/bluetooth/bluedroid/BluetoothMapSmsManager.cpp @@ +708,5 @@ > + uint16_t maxListCount = *((uint16_t *)buf); > + // convert big endian to little endian > + maxListCount = (maxListCount >> 8) | (maxListCount << 8); > + BT_LOGR("max list count: %d", maxListCount); > + AppendNamedValue(aValues, "maxListCount", maxListCount); I just found when rebasing to v2.2r, the compiler won't cast to uint32_t, it cause build failure. But I have no idea why m-c can build pass and I checked BluetoothTypes.ipdlh, it doesn't contain uint16_t type, but still can build pass, maybe compiler cast it implicitly? Anyway, I rebase it and cast it just like PBAP manager to type uint32_t. Or we shall add uint8_t/uint16_t type for BluetoothValue?

(In reply to Shawn Huang [:shawnjohnjr] from comment #20) > Anyway, I rebase it and cast it just like PBAP manager to type uint32_t. Or > we shall add uint8_t/uint16_t type for BluetoothValue? I personally prefer uint32_t casting, but I'm fine for both options.

(In reply to Ben Tian [:btian] from comment #24) > (In reply to Shawn Huang [:shawnjohnjr] from comment #20) > > Anyway, I rebase it and cast it just like PBAP manager to type uint32_t. Or > > we shall add uint8_t/uint16_t type for BluetoothValue? > > I personally prefer uint32_t casting, but I'm fine for both options. Got it. I already use uint32_t casting for both m-c and v22r patch sets.