On Fri, Jun 26, 2015 at 06:55:11PM +0300, Claudiu Olteanu wrote:
> > Just to make sure we get the terminology straight... the "local device" is
> > your computer or tablet or whatever Subsurface is running on, the "remote
> > device" is the BT dive computer, correct?
> 
> When I said the local device I was referring to the local Bluetooth device
> where the Subsurface is running on. The remote device is the dive computer
> which has support for Bluetooth.

Thanks for the clarification - that makes sense.

> > I assume that means turn on/off BT support on your computer?
> 
> Yes, that is what I meant to say. I should have been more specific.

It sometimes hard to get the wording perfect on these things. You don't
want to be too wordy - but you want to make sure it's clear especially to
someone who isn't deeply involved.

One of the things that I would like to see as part of the project
deliverables is of course documentation / addition to the user manual.
There we need to be especially careful to do this in a way that makes
sense to the non-technical person... just a diver with a computer.

> > This message seems off as a user facing message. The QtConnectivity
> > libraries would have come with the install - either as dependency or Linux
> > or bundled with the executable on Windows / Mac. What other reasons could
> > there be for this test failing? Bluetooth turned off maybe?
> > Think of this as something you tell the user so she can fix it...
>
> If the Bluetooth is turned off it wouldn't be a problem. I faced this issue
> when I played with the BlueZ and Qt versions and the Qt framework couldn't
> connect to the local Bluetooth device using the BlueZ stack. One of the
> reasons was that that the qtconnectivity library was missing or the
> BlueZ was not installed correctly.
> If the dependencies are installed correctly this should never happen.

So I wonder what a good message would be. We have a few messages that are
basically "This should never happen, please contact the Subsurface
developers and tell them <bla>" (I am paraphrasing, of course, but that's
the idea). I think that would be more appropriate.

> > Many BT UIs do that... I always wonder if it wouldn't be better NOT to
> > show the hex address if you have a name...
> 
> Probably they use the address because it is unique. Two Bluetooth devices
> may have the same name and you could easily choose the wrong one.

For the average diver... how many do you think can make any sense of that
Bluetooth address? Maybe show it if someone hovers over the name? But if
we have a name I really wouldn't show it in the UI

> > So the close function above frees the device when it closes it.
> > Is the caller required to set it to NULL? Because if someone
> > passes a device to close() (and the memory gets freed) and then
> > to read() (admittedly a bug) then you dereference freed memory
> > here.
> >
> > I guess that's just how the API works?
> 
> Yes, you are right. I should probably set the device to NULL after
> the memory was freed. For the moment this is not a problem
> because the close method is called on libdivecomputer project
> and after this happens no other operations are called.

The way the API works you would have to set the variable to NULL in the
caller - doing so in the function won't make a difference, right?

> Not having looked at the API in enough detail... but to me
> > "get_transmitted()" sounds like how much has been transmitted,
> > whereas "bytesToWrite()" sounds like a count of data in a queue...
>
> The bytesToWrite function returns the number of bytes that are
> waiting to be written. The native serial implementation uses
> the ioctl TIOCOUTQ command to get the number of bytes in the
> output buffer so I though that it would be ok if I will use
> the bytesToWrite method to get the expected results.

OK, so I simply misinterpreted the meaning of the function as used in
libdivecomputer. No problem.

> Sorry if the messages from the commits had some spelling issues
> or if they were too coincise but I spend a lot of time to resolve
> the merging conflicts and after a while I decided that it would be
> a lot easier for me to create a new branch and to commit again my
> sources :-).

Usually I find merging in git really easy, assuming you use the right
tools. Visual 3-way merge helpers like meld are really wonderful.

Overall I found the quality of your commit messages above average...

<snarkyness>
You should thank Tomaz for setting the bar low
</snarkyness>

:-)

/D

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

Reply via email to