> On Dec 1, 2017, at 7:50 AM, Murillo Bernardes <[email protected]> wrote:
> 
> On Fri, Dec 1, 2017 at 11:18 PM, Dirk Hohndel <[email protected] 
> <mailto:[email protected]>> wrote:
> So much fun to wake up to such good news.
> Thank you, Murillo!
> 
> I glanced through the diff and (like Miika) am not quite sure why you changed 
> things to QString (if this is needed, could this be a separate commit?)
> 
> Change to QStrings is not really necessary, just made it quicker for me to 
> test what I really wanted to test.

OK
 
> And of course the question is "why isn't the call to 
> connectionListMode.addAddress()" not needed?
> 
> It is needed, still there for newDC. This just removed from the list things 
> we "know" (maybe not for a fact) are not DC. I haven't understood yet all the 
> code involved, so I might be missing something.
> 
> Just to make BLE work this change is not really needed, sure.

I'm not sure I understand your answer. But then I should stare at the code in 
context some more.

> And likewise "why aren't we calling saveBtDeviceInfo(...) on the other 
> platforms".
> 
> I only found references to getBtDeviceInfo on a code specific for Mac and iOS 
> (qt-ble.cpp:300). Not sure how this works on Mac since it is never populated.
> 
> 
> It's not clear why for each device discovered, instead of just storing the 
> DeviceInfo we instead use this info to create multiple other structures (like 
> serviceUuids, btDCs) and later try to reconstruct whatever is needed.

I think it is fair to say that this code went through a lot of iterations 
between the initial ideas and what ended up working across all of the 
platforms. It may be a good idea to look into cleaning things up and 
simplifying the data structures, once we have it all working.

> I'd love to see a pull request - this makes it easy to get it tested on all 
> of our platforms. 
> 
> I don't think you've submitted code to Subsurface before - it's fairly 
> straight forward, CONTRIBUTING.md contains the basics and of course feel free 
> to ask if you need help :-)
> 
> 
> I'm working on a proper patch and will create a PR, with signed-off patches 
> and all.

Thank you. Much appreciated

/D

_______________________________________________
subsurface mailing list
[email protected]
http://lists.subsurface-divelog.org/cgi-bin/mailman/listinfo/subsurface

Reply via email to