1183496 - [nfc handover] adapter not found when pairing

Status: VERIFIED FIXED

(Firefox OS Graveyard :: Gaia::Bluetooth, defect)

Description

[[e-mail]]

on flame L & flame kk, restart the device, use nfc headset to pair with device will have highly chance to monitor `(bluetooth) adapter not found` error log and no pairing dialog is shown.

After disable/enable bluetooth, the dialog works normally.

I think it might be a 2.5 regression of bug 1094759, which might make nfc core modules loaded before bluetooth modules.

Comment 1

[[e-mail]]

Currently we only save the adapter when we receive `bluetooth-enabled` event. But with current bootstrap procedure, the NFC handover module usually start after bluetooth module, and the event is missed.

If the root cause is right, `adapter not found` will only happens when NFC & Bluetooth are default enabled after reboot. 

If we toggle Bluetooth enabled or make Bluetooth enabled automatically, the issue will not happen.

Comment 2

[GitHub Autolander Bot]

Attachment: [gaia] gasolin:issue-1183496 > mozilla-b2g:master

Comment 3

[[e-mail]]

[Blocking Requested - why for this release]: It breaks the NFC transfer

Comment 4

[Aaron Wu]

marked as 2.5+

Comment 5

[[e-mail]]

Comment on attachment 8633996 [details] [review]
[gaia] gasolin:issue-1183496 > mozilla-b2g:master

@tauzen could you help review it?

Comment 6

[Krzysztof Mioduszewski[:tauzen]]

Comment on attachment 8633996 [details] [review]
[gaia] gasolin:issue-1183496 > mozilla-b2g:master

Hi Fred, this looks good to me. I've left some small comments on github. 
I'm setting f+ and forwarding the review to Tim. I'm leaving the project, so I think I shouldn't review this.

Comment 7

[[e-mail]]

Thanks for feedback and very appreciate for your contribution! 

I've addressed your comment and would ask Tim to review this issue.

Comment 8

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 8633996 [details] [review]
[gaia] gasolin:issue-1183496 > mozilla-b2g:master

I don't believe this is a correct fix. This behaves correctly just because of the behavior of Bluetooth#adapter() method.

Could we instead always make sure there is an adopter when handling incoming NFC events? The current design is very fragile against state disparity and races like this.

If you can't do it or if I don't read the code correctly, I am happy to review this band-aid patch again.

Comment 9

[Jayme Mercado [:JMercado] (Inactive 3/4/2016)]

Fred do you still need a regression-window for this issue?  You seem to have an idea of what caused this and what is needed to fix it.

Also am I correct in understanding that when this issue occurs the NFC sharing screen will close to the homescreen?

Comment 10

[[e-mail]]

Thanks, no need regression-window here. 

The issue occurs after shrinking UI, the transfer notification will not triggered due to no bluetooth adapter.

Comment 11

[GitHub Autolander Bot]

Attachment: [gaia] gasolin:issue-1183496-2 > mozilla-b2g:master

Comment 12

[[e-mail]]

Comment on attachment 8636386 [details] [review]
[gaia] gasolin:issue-1183496-2 > mozilla-b2g:master

Always make sure there is an adopter when handling incoming NFC events. Now all _doXxxx methods are triggered after _doAction, which will request the BT adapter before execute those methods.

Test with BT on/off after restart and works well.

Comment 13

[Jayme Mercado [:JMercado] (Inactive 3/4/2016)]

Removing the regression window tag per comment 10.

Comment 14

[Tim Guan-tin Chien [:timdream] (please needinfo)]

Comment on attachment 8636386 [details] [review]
[gaia] gasolin:issue-1183496-2 > mozilla-b2g:master

Since you are changing the behavior of _doAction, please ensure this is what other methods are expecting as well.

Comment 15

[[e-mail]]

Thanks for review. I'll ask Alison for help to make sure origin operations going well in case I miss anything.

Comment 16

[[e-mail]]

Alison found some issues after applying the patch. I'll trace it later after fixed other issues.

Comment 17

[[e-mail]]

yoshi, during testing NFC on 8/5 flame pvt img,

Sending file When NFC on and BT on or off, sometimes I got logs (monitored via `adb logcat | grep "Nfc"` command):

D/nfcd    ( 3145): void NfcIpcSocket::Loop(const char*, bool):  116 of bytes to be sent... data=0x0 ret=0
D/NfcNci  ( 3145): void RouteDataSet::DeleteDatabase(): default db size=0; sec elem db size=0

After these logs, the device will turn off the screen and auto reboot. Is it a known issue?


Here is the more detailed log:

I/BrcmNfcNfa( 3145): LLCP_SendData () Local SAP:0x20, Remote SAP:0x10
D/NfcNci  ( 3145): static void PeerToPeer::NfaClientCallback(tNFA_P2P_EVT, tNFA_P2P_EVT_DATA*): enter; event=7
D/NfcNci  ( 3145): static void PeerToPeer::NfaClientCallback(tNFA_P2P_EVT, tNFA_P2P_EVT_DATA*): NFA_P2P_DATA_EVT; h=0x581; remote sap=0x10
I/BrcmNfcNfa( 3145): NFA_P2pReadData (): handle:0x581
I/BrcmNfcNfa( 3145): LLCP_ReadDataLinkData () Local SAP:0x20, Remote SAP:0x10
D/NfcNci  ( 3145): bool PeerToPeer::Receive(unsigned int, uint8_t*, uint16_t, uint16_t&): exit; nfa h: 0x581  ok: 1  actual len: 60
D/nfcd    ( 3145): void MessageHandler::ProcessResponse(NfcResponseType, NfcErrorCode, void*): enter response=2, error=0
D/nfcd    ( 3145): void NfcIpcSocket::WriteToOutgoingQueue(uint8_t*, size_t): enter, data=0xb76e8ec8, dataLen=16
D/nfcd    ( 3145): void NfcIpcSocket::WriteToOutgoingQueue(uint8_t*, size_t): Writing 16 bytes to gecko
D/nfcd    ( 3145): void* NfcService::EventLoop(): NFCService msg=14
D/nfcd    ( 3145): void MessageHandler::ProcessNotification(NfcNotificationType, void*): processNotificaton notification=4
D/nfcd    ( 3145): void NfcIpcSocket::WriteToOutgoingQueue(uint8_t*, size_t): enter, data=0xb76e8eb8, dataLen=112
D/nfcd    ( 3145): void NfcIpcSocket::WriteToOutgoingQueue(uint8_t*, size_t): Writing 112 bytes to gecko
D/nfcd    ( 3145): void NfcIpcSocket::Loop(const char*, bool):  116 of bytes to be sent... data=0x0 ret=0
D/NfcNci  ( 3145): void RouteDataSet::DeleteDatabase(): default db size=0; sec elem db size=0

Comment 18

[[e-mail]]

@alison I've re-testing between android z3c and 8/5 flame pvt img (Base img is 18Dv4),
with NFC on + BT on/off, for receive and send cases. 

Everything works except above crash (It happens during transferring so it might not related to gaia). 
Could you help verify again?


BTW, Shawn Huang told me that we should flash full pvt img after flash the Base img, so the nfc/bluetooth gonk & gecko daemon will match with each other.

Comment 19

[[e-mail]]

comment 17 is tracked in bug 1191715

Comment 20

[Alison Shiue]

I tried with bt on/off combinations, file transfer works fine.

test build:

[Flame]
Build ID               20150805150207
Gaia Revision          581de383687dc441a878d2c91a0167c6ec688fef
Gaia Date              2015-08-05 01:48:40
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/b12a261ee32e
Gecko Version          42.0a1
Device Name            flame
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.cltbld.20150805.195702
Firmware Date          Wed Aug  5 19:57:12 EDT 2015
Bootloader             L1TC000118D0

[Z3C]
Build ID               20150806003241
Gaia Revision          581de383687dc441a878d2c91a0167c6ec688fef
Gaia Date              2015-08-05 01:48:40
Gecko Revision         https://hg.mozilla.org/mozilla-central/rev/9801f91760d9
Gecko Version          42.0a1
Device Name            aries
Firmware(Release)      4.4.2
Firmware(Incremental)  eng.worker.20150805.235801
Firmware Date          Wed Aug  5 23:58:08 UTC 2015
Bootloader             s1

Comment 21

[[e-mail]]

Thanks alison,

merged to master since this patch does solve the problem https://github.com/mozilla-b2g/gaia/commit/cfbfe8e2df1ecd7710d41125bd0d2ba7f36db3af

Comment 22

[Alison Shiue]

According to Comment 20, change the status to VERIFIED FIXED.