Hi, samuel > > So we don't restrict the queue size, right? > After thinking about how deep the queue should be, and after reading > Zengguang's email, I realized that this command queue was adding more > complexity than anything else. > If we want to go for simplicity, we should probably just > return EBUSY whenever > we're not done with the current operation. Your tristate
I think returning EBUSY whenever we're not done with the current operation maybe not make sense, For it will ignore or miss the operation request, which may lead to other issues, a use case for example, I remember I submitted a patch before whose solution is not good and not be approved. I just quote it here: In current connman implementation, when we turn on/off airplane mode quickly, sometimes, it may lead to device status conflict. For example, initial status is airplane mode off, bluetooth device on, then when we turn on/off airplane mode qucikly, bluetooth device will be enabled and disabled according to it. But the bluetooth enable/disable operation is asynchronous, in some time, for example if the device is in the progress of powering on but not powered completed yet, then turn on airplane mode, which means to disable the bluetooth device, connman will treat it as the same device status and ignore the operation, then the status would be turned to airplane mode on, bluetooth device on, which can't keep consistent with the initial status. I think if each device can maintain the device status itself and record the last power request, It may fix the issue above in a nice way. So I suggest to use powered_pending flag to record the last power request and return EINPROGRESS instead of just returning EBUSY, which will ignore the power request when device is busy. So what's your suggestions about it? > powered_pending patch > could easily handle that properly, and also return EALREADY > if we get a > request for the same operation as we're currently running. > > What do you think ? > > A couple of additional comments: > 1) We need to fix the D-Bus pending message mechanism for > manager.EnableTechnology, and have that handled from the > technology layer, > with 1 pending message per technology. > 2) Your timer code is probably not needed as we're already > running a timeout > from manager.c I agree with Alok to keep timer code in his patch and remove timer in manager.c. The reason is that sometimes we enable/disable technology not only through manager enable/disable_technology Interface, for example, we can make the operation through manager setproperty(OfflineMode) as well. So maybe keep timer in device.c is better. Not sure if I have some misunderstanding about it, if yes, help me to point it out please, thanks! Best Regards, zhengguang _______________________________________________ connman mailing list [email protected] http://lists.connman.net/listinfo/connman
