B2G NFC: NFC Daemon crash sometimes when enabled

RESOLVED FIXED in Firefox OS v1.3

Status

RESOLVED FIXED
5 years ago
4 years ago

People

(Reporter: dimi, Assigned: dimi)

Tracking

unspecified
1.3 C2/1.4 S2(17jan)
x86_64
Linux
Dependency tree / graph

Firefox Tracking Flags

(blocking-b2g:1.3+, b2g-v1.3 fixed, b2g-v1.3T fixed, b2g-v1.4 fixed)

Details

Attachments

(1 attachment, 1 obsolete attachment)

59 bytes, text/x-github-pull-request
allstars.chh
: review+
Details | Review | Splinter Review
(Assignee)

Description

5 years ago
Sometimes nfc daemon will crash when keep enabling / disabling
Blocks: 860906
(Assignee)

Comment 1

5 years ago
Update current investigate result:

When we enable nfc daemon. it will do the following:

sNfcManager->initialize();
sNfcManager->enableDiscovery();

In the end of initialize function, it will set power to low level and enter sleep mode.
Then nfcd call enableDiscovery immediately after initialize.

The nci library create threads to do both sleep mode related things and enable discovery call. When the thread execution sequence is re-ordered. The nci library will crash because in-consistence of internal variable.

Android do not have this issue because android call initialize and enableDiscovery separately. So the timing issue is avoided.
(Assignee)

Comment 2

5 years ago
Proposed solution:

NFC Daemon:
 - Separate enableNfc and enableDiscovery API. enableNfc from upper layer do not trigger enableDiscovery

GAIA:
 - After NFC is enabled, need check screen state to call enable/disable discovery accordingly
(Assignee)

Updated

5 years ago
Flags: needinfo?(dgarnerlee)
(Assignee)

Comment 3

5 years ago
Hi Garner,
  Could you help check if it is ok to check screen state in GAIA part and set enable/disable discovery according to current state ?

Comment 4

5 years ago
Hi Dimi,

I'll check. The system nfc_manager.js (in the SVIC Gaia master branch) does currently check screen states (boolean screen locked, and boolean sceen off), to determine enable/disable states. That state can be forwarded to setConfig as a new PDU (1.7 protocol currently only has powerlevel=1, which isn't actually used anywhere yet and can be easily be extended/redefined). Is that value not specific enough to resolve?
Flags: needinfo?(dgarnerlee)
(Assignee)

Comment 5

5 years ago
(In reply to Garner Lee from comment #4)
> Hi Dimi,
> 
> I'll check. The system nfc_manager.js (in the SVIC Gaia master branch) does
> currently check screen states (boolean screen locked, and boolean sceen
> off), to determine enable/disable states. That state can be forwarded to
> setConfig as a new PDU (1.7 protocol currently only has powerlevel=1, which
> isn't actually used anywhere yet and can be easily be extended/redefined).
> Is that value not specific enough to resolve?

As the discuss result with Arno during Taipei work week.
The configRequest may have 3 value:
1. enable
2. disableDiscovery
3. disable

But since the root cause of this bug is that when UI call enable, nfcd will do both enable and enableDiscovery.
I will suggest that configRequest should add enableDiscovery. So it will have following values
1. enable
2. enableDiscovery
3. disable
4. disableDiscovery

The use case will be :
- When user turn on NFC setting, GAIA should call enable.
- When user turn off NFC setting, GAIA should call disable.
- When NFC is on, GAIA should check current screen state and call enableDiscovery/disableDiscovery accordingly.

How do you think ?
(Assignee)

Updated

5 years ago
Flags: needinfo?(arno)

Updated

5 years ago
blocking-b2g: --- → 1.3+

Comment 6

5 years ago
(In reply to Dimi Lee [:dlee] from comment #5)
> (In reply to Garner Lee from comment #4)
> > Hi Dimi,
> > 
> > I'll check. The system nfc_manager.js (in the SVIC Gaia master branch) does
> > currently check screen states (boolean screen locked, and boolean sceen
> > off), to determine enable/disable states. That state can be forwarded to
> > setConfig as a new PDU (1.7 protocol currently only has powerlevel=1, which
> > isn't actually used anywhere yet and can be easily be extended/redefined).
> > Is that value not specific enough to resolve?
> 
> As the discuss result with Arno during Taipei work week.
> The configRequest may have 3 value:
> 1. enable
> 2. disableDiscovery
> 3. disable
> 
> But since the root cause of this bug is that when UI call enable, nfcd will
> do both enable and enableDiscovery.
> I will suggest that configRequest should add enableDiscovery. So it will
> have following values
> 1. enable
> 2. enableDiscovery
> 3. disable
> 4. disableDiscovery
> 
> The use case will be :
> - When user turn on NFC setting, GAIA should call enable.
> - When user turn off NFC setting, GAIA should call disable.
> - When NFC is on, GAIA should check current screen state and call
> enableDiscovery/disableDiscovery accordingly.
> 
> How do you think ?

sorry for not responding earlier to this. I updated the protocol spec according to our discussion in Taipei. I wonder if the three states are sufficient:

- when NFC is OFF in settings, send 0
- when NFC is turned on in settings, send 2
- when screen is turned off (and NFC is still turned on in settings), send 1
- when screen is turned on AND unlocked, send 2

Values 0, 1, 2 are according to the protocol spec.

Do you think this would work?

Arno
Flags: needinfo?(arno)
(Assignee)

Comment 7

5 years ago
(In reply to arno from comment #6)
> (In reply to Dimi Lee [:dlee] from comment #5)
> > (In reply to Garner Lee from comment #4)
> > > Hi Dimi,
> > > 
> > > I'll check. The system nfc_manager.js (in the SVIC Gaia master branch) does
> > > currently check screen states (boolean screen locked, and boolean sceen
> > > off), to determine enable/disable states. That state can be forwarded to
> > > setConfig as a new PDU (1.7 protocol currently only has powerlevel=1, which
> > > isn't actually used anywhere yet and can be easily be extended/redefined).
> > > Is that value not specific enough to resolve?
> > 
> > As the discuss result with Arno during Taipei work week.
> > The configRequest may have 3 value:
> > 1. enable
> > 2. disableDiscovery
> > 3. disable
> > 
> > But since the root cause of this bug is that when UI call enable, nfcd will
> > do both enable and enableDiscovery.
> > I will suggest that configRequest should add enableDiscovery. So it will
> > have following values
> > 1. enable
> > 2. enableDiscovery
> > 3. disable
> > 4. disableDiscovery
> > 
> > The use case will be :
> > - When user turn on NFC setting, GAIA should call enable.
> > - When user turn off NFC setting, GAIA should call disable.
> > - When NFC is on, GAIA should check current screen state and call
> > enableDiscovery/disableDiscovery accordingly.
> > 
> > How do you think ?
> 
> sorry for not responding earlier to this. I updated the protocol spec
> according to our discussion in Taipei. I wonder if the three states are
> sufficient:
> 
> - when NFC is OFF in settings, send 0
> - when NFC is turned on in settings, send 2
> - when screen is turned off (and NFC is still turned on in settings), send 1
> - when screen is turned on AND unlocked, send 2
> 
> Values 0, 1, 2 are according to the protocol spec.
> 
> Do you think this would work?
> 
> Arno

There are some problem for case "when NFC is turned on in settings, send 2" in current nfcd implementation.
The problem is the result of this bug, it cause exception sometime when enable NFC. And the root cause is written in Comment 1.

I suggest modify current spec and add state 3 (NFC chip is on).
So for case "when NFC is turned on in settings", GAIA will do the following:
1. send 3 (NFC chip is on)  --> nfc daemon initialize library but do not enable discovery.
2. check screen state if it is on, send 2 (NFC chip is on and enable discovery)  --> nfc daemon will enable discovery.

If we do not add one more value, it will then become when turning on NFC, GAIA will send 2 twice.
Flags: needinfo?(arno)

Comment 8

5 years ago
(In reply to Dimi Lee [:dlee] from comment #7)
> (In reply to arno from comment #6)
> > (In reply to Dimi Lee [:dlee] from comment #5)
> > > (In reply to Garner Lee from comment #4)
> > > > Hi Dimi,
> > > > 
> > > > I'll check. The system nfc_manager.js (in the SVIC Gaia master branch) does
> > > > currently check screen states (boolean screen locked, and boolean sceen
> > > > off), to determine enable/disable states. That state can be forwarded to
> > > > setConfig as a new PDU (1.7 protocol currently only has powerlevel=1, which
> > > > isn't actually used anywhere yet and can be easily be extended/redefined).
> > > > Is that value not specific enough to resolve?
> > > 
> > > As the discuss result with Arno during Taipei work week.
> > > The configRequest may have 3 value:
> > > 1. enable
> > > 2. disableDiscovery
> > > 3. disable
> > > 
> > > But since the root cause of this bug is that when UI call enable, nfcd will
> > > do both enable and enableDiscovery.
> > > I will suggest that configRequest should add enableDiscovery. So it will
> > > have following values
> > > 1. enable
> > > 2. enableDiscovery
> > > 3. disable
> > > 4. disableDiscovery
> > > 
> > > The use case will be :
> > > - When user turn on NFC setting, GAIA should call enable.
> > > - When user turn off NFC setting, GAIA should call disable.
> > > - When NFC is on, GAIA should check current screen state and call
> > > enableDiscovery/disableDiscovery accordingly.
> > > 
> > > How do you think ?
> > 
> > sorry for not responding earlier to this. I updated the protocol spec
> > according to our discussion in Taipei. I wonder if the three states are
> > sufficient:
> > 
> > - when NFC is OFF in settings, send 0
> > - when NFC is turned on in settings, send 2
> > - when screen is turned off (and NFC is still turned on in settings), send 1
> > - when screen is turned on AND unlocked, send 2
> > 
> > Values 0, 1, 2 are according to the protocol spec.
> > 
> > Do you think this would work?
> > 
> > Arno
> 
> There are some problem for case "when NFC is turned on in settings, send 2"
> in current nfcd implementation.
> The problem is the result of this bug, it cause exception sometime when
> enable NFC. And the root cause is written in Comment 1.
> 
> I suggest modify current spec and add state 3 (NFC chip is on).
> So for case "when NFC is turned on in settings", GAIA will do the following:
> 1. send 3 (NFC chip is on)  --> nfc daemon initialize library but do not
> enable discovery.
> 2. check screen state if it is on, send 2 (NFC chip is on and enable
> discovery)  --> nfc daemon will enable discovery.
> 
> If we do not add one more value, it will then become when turning on NFC,
> GAIA will send 2 twice.

When NFC is turned on via Settings, the screen must be on and unlocked. I don't know why Gaia is sending 2 twice, but that sounds like a bug. In case Gaia only sends the value of 2 once, would that be sufficient?

If you think not, what should the semantics of "3" be? "Initialize NFC chip"? We probably need a mini state machine for the 4 different values to capture all the legal transitions.

Arno
Flags: needinfo?(arno)
(Assignee)

Comment 9

5 years ago
(In reply to arno from comment #8)
> (In reply to Dimi Lee [:dlee] from comment #7)
> > (In reply to arno from comment #6)
> > > (In reply to Dimi Lee [:dlee] from comment #5)
> > > > (In reply to Garner Lee from comment #4)
> > > > > Hi Dimi,
> > > > > 
> > > > > I'll check. The system nfc_manager.js (in the SVIC Gaia master branch) does
> > > > > currently check screen states (boolean screen locked, and boolean sceen
> > > > > off), to determine enable/disable states. That state can be forwarded to
> > > > > setConfig as a new PDU (1.7 protocol currently only has powerlevel=1, which
> > > > > isn't actually used anywhere yet and can be easily be extended/redefined).
> > > > > Is that value not specific enough to resolve?
> > > > 
> > > > As the discuss result with Arno during Taipei work week.
> > > > The configRequest may have 3 value:
> > > > 1. enable
> > > > 2. disableDiscovery
> > > > 3. disable
> > > > 
> > > > But since the root cause of this bug is that when UI call enable, nfcd will
> > > > do both enable and enableDiscovery.
> > > > I will suggest that configRequest should add enableDiscovery. So it will
> > > > have following values
> > > > 1. enable
> > > > 2. enableDiscovery
> > > > 3. disable
> > > > 4. disableDiscovery
> > > > 
> > > > The use case will be :
> > > > - When user turn on NFC setting, GAIA should call enable.
> > > > - When user turn off NFC setting, GAIA should call disable.
> > > > - When NFC is on, GAIA should check current screen state and call
> > > > enableDiscovery/disableDiscovery accordingly.
> > > > 
> > > > How do you think ?
> > > 
> > > sorry for not responding earlier to this. I updated the protocol spec
> > > according to our discussion in Taipei. I wonder if the three states are
> > > sufficient:
> > > 
> > > - when NFC is OFF in settings, send 0
> > > - when NFC is turned on in settings, send 2
> > > - when screen is turned off (and NFC is still turned on in settings), send 1
> > > - when screen is turned on AND unlocked, send 2
> > > 
> > > Values 0, 1, 2 are according to the protocol spec.
> > > 
> > > Do you think this would work?
> > > 
> > > Arno
> > 
> > There are some problem for case "when NFC is turned on in settings, send 2"
> > in current nfcd implementation.
> > The problem is the result of this bug, it cause exception sometime when
> > enable NFC. And the root cause is written in Comment 1.
> > 
> > I suggest modify current spec and add state 3 (NFC chip is on).
> > So for case "when NFC is turned on in settings", GAIA will do the following:
> > 1. send 3 (NFC chip is on)  --> nfc daemon initialize library but do not
> > enable discovery.
> > 2. check screen state if it is on, send 2 (NFC chip is on and enable
> > discovery)  --> nfc daemon will enable discovery.
> > 
> > If we do not add one more value, it will then become when turning on NFC,
> > GAIA will send 2 twice.
> 
> When NFC is turned on via Settings, the screen must be on and unlocked. I
> don't know why Gaia is sending 2 twice, but that sounds like a bug. In case
> Gaia only sends the value of 2 once, would that be sufficient?
> 
> If you think not, what should the semantics of "3" be? "Initialize NFC
> chip"? We probably need a mini state machine for the 4 different values to
> capture all the legal transitions.
> 
> Arno

Sorry for misleading, the reason GAIA send 2 twice is based on my proposed solution for this bug, not current implementation.

In current implementation, GAIA send 2 once is not sufficient. nfcd do initialize and enable discovery in one API call will cause crash sometimes. The reason is written in Comment 1, and android do not have this issue is because android separate initialize and enable discovery.

So yes i would propose add "3" to configRequest and the semantics will like "Initialize NFC chip".

Comment 10

5 years ago
It seems better to finish this bug before sprint5. If you think it isn't reasonable, please update target milestone.

Comment 11

5 years ago
(In reply to Dimi Lee [:dlee] from comment #9)
> (In reply to arno from comment #8)
> > (In reply to Dimi Lee [:dlee] from comment #7)
> > > (In reply to arno from comment #6)
> > > > (In reply to Dimi Lee [:dlee] from comment #5)
> > > > > (In reply to Garner Lee from comment #4)
> > > > > > Hi Dimi,
> > > > > > 
> > > > > > I'll check. The system nfc_manager.js (in the SVIC Gaia master branch) does
> > > > > > currently check screen states (boolean screen locked, and boolean sceen
> > > > > > off), to determine enable/disable states. That state can be forwarded to
> > > > > > setConfig as a new PDU (1.7 protocol currently only has powerlevel=1, which
> > > > > > isn't actually used anywhere yet and can be easily be extended/redefined).
> > > > > > Is that value not specific enough to resolve?
> > > > > 
> > > > > As the discuss result with Arno during Taipei work week.
> > > > > The configRequest may have 3 value:
> > > > > 1. enable
> > > > > 2. disableDiscovery
> > > > > 3. disable
> > > > > 
> > > > > But since the root cause of this bug is that when UI call enable, nfcd will
> > > > > do both enable and enableDiscovery.
> > > > > I will suggest that configRequest should add enableDiscovery. So it will
> > > > > have following values
> > > > > 1. enable
> > > > > 2. enableDiscovery
> > > > > 3. disable
> > > > > 4. disableDiscovery
> > > > > 
> > > > > The use case will be :
> > > > > - When user turn on NFC setting, GAIA should call enable.
> > > > > - When user turn off NFC setting, GAIA should call disable.
> > > > > - When NFC is on, GAIA should check current screen state and call
> > > > > enableDiscovery/disableDiscovery accordingly.
> > > > > 
> > > > > How do you think ?
> > > > 
> > > > sorry for not responding earlier to this. I updated the protocol spec
> > > > according to our discussion in Taipei. I wonder if the three states are
> > > > sufficient:
> > > > 
> > > > - when NFC is OFF in settings, send 0
> > > > - when NFC is turned on in settings, send 2
> > > > - when screen is turned off (and NFC is still turned on in settings), send 1
> > > > - when screen is turned on AND unlocked, send 2
> > > > 
> > > > Values 0, 1, 2 are according to the protocol spec.
> > > > 
> > > > Do you think this would work?
> > > > 
> > > > Arno
> > > 
> > > There are some problem for case "when NFC is turned on in settings, send 2"
> > > in current nfcd implementation.
> > > The problem is the result of this bug, it cause exception sometime when
> > > enable NFC. And the root cause is written in Comment 1.
> > > 
> > > I suggest modify current spec and add state 3 (NFC chip is on).
> > > So for case "when NFC is turned on in settings", GAIA will do the following:
> > > 1. send 3 (NFC chip is on)  --> nfc daemon initialize library but do not
> > > enable discovery.
> > > 2. check screen state if it is on, send 2 (NFC chip is on and enable
> > > discovery)  --> nfc daemon will enable discovery.
> > > 
> > > If we do not add one more value, it will then become when turning on NFC,
> > > GAIA will send 2 twice.
> > 
> > When NFC is turned on via Settings, the screen must be on and unlocked. I
> > don't know why Gaia is sending 2 twice, but that sounds like a bug. In case
> > Gaia only sends the value of 2 once, would that be sufficient?
> > 
> > If you think not, what should the semantics of "3" be? "Initialize NFC
> > chip"? We probably need a mini state machine for the 4 different values to
> > capture all the legal transitions.
> > 
> > Arno
> 
> Sorry for misleading, the reason GAIA send 2 twice is based on my proposed
> solution for this bug, not current implementation.
> 
> In current implementation, GAIA send 2 once is not sufficient. nfcd do
> initialize and enable discovery in one API call will cause crash sometimes.
> The reason is written in Comment 1, and android do not have this issue is
> because android separate initialize and enable discovery.
> 
> So yes i would propose add "3" to configRequest and the semantics will like
> "Initialize NFC chip".

ok, before we make that change, would it perhaps be better to break it up into two config parameters again as we initially discussed? "powerLevel" (off/on) and "discoveryMode" (off/on)?
(Assignee)

Comment 12

5 years ago
(In reply to arno from comment #11)
> (In reply to Dimi Lee [:dlee] from comment #9)
> > (In reply to arno from comment #8)
> > > (In reply to Dimi Lee [:dlee] from comment #7)
> > > > (In reply to arno from comment #6)
> > > > > (In reply to Dimi Lee [:dlee] from comment #5)
> > > > > > (In reply to Garner Lee from comment #4)
> > > > > > > Hi Dimi,
> > > > > > > 
> > > > > > > I'll check. The system nfc_manager.js (in the SVIC Gaia master branch) does
> > > > > > > currently check screen states (boolean screen locked, and boolean sceen
> > > > > > > off), to determine enable/disable states. That state can be forwarded to
> > > > > > > setConfig as a new PDU (1.7 protocol currently only has powerlevel=1, which
> > > > > > > isn't actually used anywhere yet and can be easily be extended/redefined).
> > > > > > > Is that value not specific enough to resolve?
> > > > > > 
> > > > > > As the discuss result with Arno during Taipei work week.
> > > > > > The configRequest may have 3 value:
> > > > > > 1. enable
> > > > > > 2. disableDiscovery
> > > > > > 3. disable
> > > > > > 
> > > > > > But since the root cause of this bug is that when UI call enable, nfcd will
> > > > > > do both enable and enableDiscovery.
> > > > > > I will suggest that configRequest should add enableDiscovery. So it will
> > > > > > have following values
> > > > > > 1. enable
> > > > > > 2. enableDiscovery
> > > > > > 3. disable
> > > > > > 4. disableDiscovery
> > > > > > 
> > > > > > The use case will be :
> > > > > > - When user turn on NFC setting, GAIA should call enable.
> > > > > > - When user turn off NFC setting, GAIA should call disable.
> > > > > > - When NFC is on, GAIA should check current screen state and call
> > > > > > enableDiscovery/disableDiscovery accordingly.
> > > > > > 
> > > > > > How do you think ?
> > > > > 
> > > > > sorry for not responding earlier to this. I updated the protocol spec
> > > > > according to our discussion in Taipei. I wonder if the three states are
> > > > > sufficient:
> > > > > 
> > > > > - when NFC is OFF in settings, send 0
> > > > > - when NFC is turned on in settings, send 2
> > > > > - when screen is turned off (and NFC is still turned on in settings), send 1
> > > > > - when screen is turned on AND unlocked, send 2
> > > > > 
> > > > > Values 0, 1, 2 are according to the protocol spec.
> > > > > 
> > > > > Do you think this would work?
> > > > > 
> > > > > Arno
> > > > 
> > > > There are some problem for case "when NFC is turned on in settings, send 2"
> > > > in current nfcd implementation.
> > > > The problem is the result of this bug, it cause exception sometime when
> > > > enable NFC. And the root cause is written in Comment 1.
> > > > 
> > > > I suggest modify current spec and add state 3 (NFC chip is on).
> > > > So for case "when NFC is turned on in settings", GAIA will do the following:
> > > > 1. send 3 (NFC chip is on)  --> nfc daemon initialize library but do not
> > > > enable discovery.
> > > > 2. check screen state if it is on, send 2 (NFC chip is on and enable
> > > > discovery)  --> nfc daemon will enable discovery.
> > > > 
> > > > If we do not add one more value, it will then become when turning on NFC,
> > > > GAIA will send 2 twice.
> > > 
> > > When NFC is turned on via Settings, the screen must be on and unlocked. I
> > > don't know why Gaia is sending 2 twice, but that sounds like a bug. In case
> > > Gaia only sends the value of 2 once, would that be sufficient?
> > > 
> > > If you think not, what should the semantics of "3" be? "Initialize NFC
> > > chip"? We probably need a mini state machine for the 4 different values to
> > > capture all the legal transitions.
> > > 
> > > Arno
> > 
> > Sorry for misleading, the reason GAIA send 2 twice is based on my proposed
> > solution for this bug, not current implementation.
> > 
> > In current implementation, GAIA send 2 once is not sufficient. nfcd do
> > initialize and enable discovery in one API call will cause crash sometimes.
> > The reason is written in Comment 1, and android do not have this issue is
> > because android separate initialize and enable discovery.
> > 
> > So yes i would propose add "3" to configRequest and the semantics will like
> > "Initialize NFC chip".
> 
> ok, before we make that change, would it perhaps be better to break it up
> into two config parameters again as we initially discussed? "powerLevel"
> (off/on) and "discoveryMode" (off/on)?

If we use two parameter, i will assume there may have two case:
1.
- powerLevel : on, discoveryMode : off   -> initialize NFC chip
- powerLevel : on, discoveryMode : on    -> enable discovery
- powerLevel : off, discoveryMode : off  -> uninitializze NFC chip
- powerLevel : off, discoveryMode : on   -> in-correct parameter
No disable discovery

2.
- powerLevel : on, discoveryMode : off   -> disable discovery
- powerLevel : on, discoveryMode : on    -> initialize NFC chip
- powerLevel : off, discoveryMode : off  -> uninitializze NFC chip
- powerLevel : off, discoveryMode : on   -> in-correct parameter
No enable discovery

Both case loss one information because "power off/discovery mode on" is useless, so there is still only 3 parameters.
So i will suggest use one parameter with
- initialize NFC chip
- un-initialize NFC chip
- enable discovery
- disable discovery

Comment 13

5 years ago
(In reply to Dimi Lee [:dlee] from comment #12)
> If we use two parameter, i will assume there may have two case:
> 1.
> - powerLevel : on, discoveryMode : off   -> initialize NFC chip
> - powerLevel : on, discoveryMode : on    -> enable discovery
> - powerLevel : off, discoveryMode : off  -> uninitializze NFC chip
> - powerLevel : off, discoveryMode : on   -> in-correct parameter
> No disable discovery
> 
> 2.
> - powerLevel : on, discoveryMode : off   -> disable discovery
> - powerLevel : on, discoveryMode : on    -> initialize NFC chip
> - powerLevel : off, discoveryMode : off  -> uninitializze NFC chip
> - powerLevel : off, discoveryMode : on   -> in-correct parameter
> No enable discovery
> 
> Both case loss one information because "power off/discovery mode on" is
> useless, so there is still only 3 parameters.
> So i will suggest use one parameter with
> - initialize NFC chip
> - un-initialize NFC chip
> - enable discovery
> - disable discovery

sounds ok. Another benefit is that it requires fewer changes considering the time pressure...

Comment 14

5 years ago
I've updated the protocol spec and added a fourth value. I've also added some notes when to use each of the hardwareStates. Please take a look. We need to synchronize the change to be consistent with our code bases. Dimi: send us a ping when you made the change.
(Assignee)

Comment 15

5 years ago
Hi arno,
 I have commit the change to https://github.com/dleeMozilla/nfcd master branch
 Please help check if it could work properly after GAIA part is implemented.

 Thanks!
Flags: needinfo?(arno)
(Assignee)

Updated

5 years ago
Flags: needinfo?(arno)

Comment 16

5 years ago
(In reply to Dimi Lee [:dlee] from comment #15)
> Hi arno,
>  I have commit the change to https://github.com/dleeMozilla/nfcd master
> branch
>  Please help check if it could work properly after GAIA part is implemented.
> 
>  Thanks!

Sid is working on this right now. He'll reply to this comment when he made the change.

Updated

5 years ago
Blocks: 860910

Comment 17

5 years ago
(In reply to arno from comment #16)
> (In reply to Dimi Lee [:dlee] from comment #15)
> > Hi arno,
> >  I have commit the change to https://github.com/dleeMozilla/nfcd master
> > branch
> >  Please help check if it could work properly after GAIA part is implemented.
> > 
> >  Thanks!
> 
> Sid is working on this right now. He'll reply to this comment when he made
> the change.
Should we close this bug and create another bug for tracking Gaia part?
Flags: needinfo?(arno)
(Assignee)

Comment 18

5 years ago
I test modified gecko and nfcd.
This issue still can be reproduced. I will check more detail to find if there is a better solution than current solution (Remove set low power in initial API).

By the way, in our use case now, current solution won't affect anything because we always enable discovery immediately after calling initial. So set low power in initial API can be ignored.
Flags: needinfo?(arno)

Comment 19

5 years ago
Dimi, do you have any solution for this bug?
Flags: needinfo?(dlee)
Target Milestone: 1.3 Sprint 4 - 11/8 → ---
(Assignee)

Comment 20

5 years ago
Created attachment 8349893 [details] [diff] [review]
Proposed solution

The crash happen inside libnfc-nci. If we call power off then call power on immediately, it will cause libnfc-nci crash.
So set a delay between power off and power on.
Attachment #8349893 - Flags: feedback?(allstars.chh)
Flags: needinfo?(dlee)
Attachment #8349893 - Flags: feedback?(allstars.chh) → feedback+
(Assignee)

Comment 21

5 years ago
Created attachment 8359020 [details] [review]
Bug 934835 Fix V1
Attachment #8349893 - Attachment is obsolete: true
Attachment #8359020 - Flags: review?(allstars.chh)
(Assignee)

Updated

5 years ago
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
John, can you please assist with the uplift? :)
Flags: needinfo?(jhford)
The nfcd repository doesn't have a v1.3 branch.  This repository is only used for nexus-4.xml on v1.3, and that is baked to point at the nfcd 'master' branch.

As it stands, the v1.3 builds already contain this code so there is no substantive change to happen here.  This would also be the first branch in the nfcd repository.

Dimi: should we create a v1.3 branch in the nfcd project or should we continue to point it at the master branch for the time being?
Flags: needinfo?(jhford) → needinfo?(dlee)
(Assignee)

Comment 25

5 years ago
(In reply to John Ford [:jhford] -- please use 'needinfo?' instead of a CC from comment #24)
> The nfcd repository doesn't have a v1.3 branch.  This repository is only
> used for nexus-4.xml on v1.3, and that is baked to point at the nfcd
> 'master' branch.
> 
> As it stands, the v1.3 builds already contain this code so there is no
> substantive change to happen here.  This would also be the first branch in
> the nfcd repository.
> 
> Dimi: should we create a v1.3 branch in the nfcd project or should we
> continue to point it at the master branch for the time being?

I would suggest point at the master branch for the time being.
Flags: needinfo?(dlee)
status-b2g-v1.3: --- → fixed
status-b2g-v1.4: --- → fixed
Blocks: 1044425
No longer blocks: 860906
status-b2g-v1.3T: --- → fixed
Target Milestone: --- → 1.3 C2/1.4 S2(17jan)
You need to log in before you can comment on or make changes to this bug.