Re: [PATCH 0/2] Clean up some of the dive computer nickname code
On Wed, Jan 9, 2013 at 12:31 PM, Linus Torvalds <torvalds@linux-foundation.org> wrote:
This is a small 2-patch series that makes the dive computer nickname infrastructure be a more generic "device info" structure instead, and splits it out into "device.[ch]".
Side note: as discussed with Dirk, the intent of this series is that we can save other information about the dive computer (serial numbers etc) in the device info structure. However, quite frankly, looking at what we get at least right now from libdivecomputer, I doubt it is worth it. The "serial nr" thing is an integer, which is unlikely to actually match any serial numbering on the package or anything like that. For example, the Suunto serial number that libdivecomputer gives me for the helo2 is 389152795. The *actual* serial number of the device is "23500027". Guess what? Serial numbers are strings, not numbers. Look around, you'll notice that manufacturers almost always encode serial "numbers" with letters etc. The libdivecomputer interface is wrong. It so happens that the Suunto serial number strings are strings that have all numbers, but they aren't *one* number. They are four bytes representing two numbers each, and the "23500027" string is actually the four bytes 23 50 00 27 (0x17 0x32 0x00 0x1b). And libdivecomputer has incorrectly parsed those four bytes as one number, not as the encoded serial number string it is. So the value 389152795 is actually hex 0x1732001b, which is 0x17 0x32 0x00 0x1b, which is - 23 50 00 27. IOW, the serial numbers libdivecomputer gives us are useless. You could perhaps turn them into an actual serial number string on a per-vendor (or per-device) basis, if you know the vendor rules. The Suunto vendor rules *seem* to be pretty simple, based on the two devices I have (the Vyper Air clearly uses the same encoding, giving another 8-byte string). Other dive computers? Who knows? Anyway, the current libdivecomputer "serial number" information is wrong. It has some information, but the information is in the wrong format, and you need vendor-specific knowledge to turn it into the right string. As a result, there's no point in having subsurface save it. We can use the integer form for the device ID hash, and that's all it is actually useful for. It would be good if libdivecomputer continued to give the old "numeric serial number" for that use, but to make it *useful* we also need some interface to get the actual vendor string representation, so that we can save that. If we get the correct serial numbers in the proper format that the vendor uses, *that* information wou;ld be useful to save for things like warranty purposes etc. It could be as simple as a new libdivecomputer callback ("given this serial *number*, what is the string for it?"), but it would probably be better to expand the current DC_EVENT_DEVINFO event to simply give us a string in addition to the number. Because it's not necessarily the case that all serial number strings can even be represented as a single 32-bit integer. Linus
On Wed, Jan 9, 2013 at 3:10 PM, Linus Torvalds <torvalds@linux-foundation.org> wrote:
IOW, the serial numbers libdivecomputer gives us are useless. You could perhaps turn them into an actual serial number string on a per-vendor (or per-device) basis, if you know the vendor rules. The Suunto vendor rules *seem* to be pretty simple, based on the two devices I have (the Vyper Air clearly uses the same encoding, giving another 8-byte string). Other dive computers? Who knows?
Here's a patch for subsurface that hacks in the Suunto rules, and can thus save serial number and firmware version information for Suunto dive computers. It should be done by libdivecomputer, but hey, in the meantime this at least shows the concept. And helps test the XML save/restore code. It depends on the two patches that create the whole "device.c" infrastructure, of course. With this, my dive file ends up having the settings section look like this: <divecomputerid model='Suunto Vyper Air' deviceid='d4629110' serial='01201094' firmware='1.1.22'/> <divecomputerid model='Suunto HelO2' deviceid='995dd566' serial='23500027' firmware='1.0.4'/> where the format of the firmware version is something I guessed at, but it was the obvious choice (again, it's byte-based, I'm ignoring the high byte that is zero for both of my Suuntos). Linus
Linus Torvalds <torvalds@linux-foundation.org> writes:
On Wed, Jan 9, 2013 at 3:10 PM, Linus Torvalds <torvalds@linux-foundation.org> wrote:
IOW, the serial numbers libdivecomputer gives us are useless. You could perhaps turn them into an actual serial number string on a per-vendor (or per-device) basis, if you know the vendor rules. The Suunto vendor rules *seem* to be pretty simple, based on the two devices I have (the Vyper Air clearly uses the same encoding, giving another 8-byte string). Other dive computers? Who knows?
Here's a patch for subsurface that hacks in the Suunto rules, and can thus save serial number and firmware version information for Suunto dive computers.
It should be done by libdivecomputer, but hey, in the meantime this at least shows the concept. And helps test the XML save/restore code.
It depends on the two patches that create the whole "device.c" infrastructure, of course. With this, my dive file ends up having the settings section look like this:
<divecomputerid model='Suunto Vyper Air' deviceid='d4629110' serial='01201094' firmware='1.1.22'/> <divecomputerid model='Suunto HelO2' deviceid='995dd566' serial='23500027' firmware='1.0.4'/>
where the format of the firmware version is something I guessed at, but it was the obvious choice (again, it's byte-based, I'm ignoring the high byte that is zero for both of my Suuntos).
I like this. Yes, we will have to tackle them one by one as we learn how to do this (or until libdivecomputer does it for us), but still, I like being able to save this information. Is there a reason this didn't have a commit message / SOB ? /D
On Wed, Jan 9, 2013 at 4:23 PM, Dirk Hohndel <dirk@hohndel.org> wrote:
Is there a reason this didn't have a commit message / SOB ?
I guess I'm ok with it being merged, because the result is fine, and the Suunto hack is pretty isolated and not too disgusting. So go ahead and add a sign-off. I have to run out to drive kids, so no good commit message. Maybe you can cobble something together from the previous email. Linus
On Wed, Jan 9, 2013 at 4:30 PM, Linus Torvalds <torvalds@linux-foundation.org> wrote:
I guess I'm ok with it being merged, because the result is fine, and the Suunto hack is pretty isolated and not too disgusting. So go ahead and add a sign-off.
Oh, you should probably check that it works with your suunto too. I may have two different suuntos, but they are both the same "vyper2" back-end, so they are actually fairly similar. I *suspect* this is a generic suunto format, but still.. The way the code works, you don't even have to download any actual new dives: you just need to connect, see that the dive already exists, and then do a ^S to save the end result. The divecomputer serial number should have been picked up by then already, and automatically added and saved to the xml file. Linus
On 2013-01-10 00:10, Linus Torvalds wrote:
Side note: as discussed with Dirk, the intent of this series is that we can save other information about the dive computer (serial numbers etc) in the device info structure.
However, quite frankly, looking at what we get at least right now from libdivecomputer, I doubt it is worth it. The "serial nr" thing is an integer, which is unlikely to actually match any serial numbering on the package or anything like that.
For example, the Suunto serial number that libdivecomputer gives me for the helo2 is 389152795. The *actual* serial number of the device is "23500027".
Guess what? Serial numbers are strings, not numbers. Look around, you'll notice that manufacturers almost always encode serial "numbers" with letters etc. The libdivecomputer interface is wrong.
It so happens that the Suunto serial number strings are strings that have all numbers, but they aren't *one* number. They are four bytes representing two numbers each, and the "23500027" string is actually the four bytes 23 50 00 27 (0x17 0x32 0x00 0x1b). And libdivecomputer has incorrectly parsed those four bytes as one number, not as the encoded serial number string it is. So the value 389152795 is actually hex 0x1732001b, which is 0x17 0x32 0x00 0x1b, which is - 23 50 00 27.
IOW, the serial numbers libdivecomputer gives us are useless. You could perhaps turn them into an actual serial number string on a per-vendor (or per-device) basis, if you know the vendor rules. The Suunto vendor rules *seem* to be pretty simple, based on the two devices I have (the Vyper Air clearly uses the same encoding, giving another 8-byte string). Other dive computers? Who knows?
Anyway, the current libdivecomputer "serial number" information is wrong. It has some information, but the information is in the wrong format, and you need vendor-specific knowledge to turn it into the right string.
As a result, there's no point in having subsurface save it. We can use the integer form for the device ID hash, and that's all it is actually useful for. It would be good if libdivecomputer continued to give the old "numeric serial number" for that use, but to make it *useful* we also need some interface to get the actual vendor string representation, so that we can save that. If we get the correct serial numbers in the proper format that the vendor uses, *that* information wou;ld be useful to save for things like warranty purposes etc.
It could be as simple as a new libdivecomputer callback ("given this serial *number*, what is the string for it?"), but it would probably be better to expand the current DC_EVENT_DEVINFO event to simply give us a string in addition to the number. Because it's not necessarily the case that all serial number strings can even be represented as a single 32-bit integer.
The primary purpose of the serial number in the DC_EVENT_DEVINFO event is to be able to uniquely identify individual devices (e.g. for use with the fingerprint feature). For this purpose, it doesn't really matter if the serial number (as returned by libdivecomputer) doesn't match the real serial number (as printed on the hardware). That's why I didn't bother to decode it properly, and just return the raw bytes wrapped in a 32bit integer. In this sense the serial number is more like an opaque device id, and it was not really intended to be shown to the end-user (e.g. for warranty purposes). Until now, all serial numbers did nicely fit into a 32bit integer, but if there ever happens to be a device where this isn't possible, we can always calculate some hash, just like you already do in subsurface. But of course that will only work if we make no guarantees that the libdivecomputer serial number will match the real serial number. Jef
Jef Driesen <jefdriesen@telenet.be> writes:
It could be as simple as a new libdivecomputer callback ("given this serial *number*, what is the string for it?"), but it would probably be better to expand the current DC_EVENT_DEVINFO event to simply give us a string in addition to the number. Because it's not necessarily the case that all serial number strings can even be represented as a single 32-bit integer.
The primary purpose of the serial number in the DC_EVENT_DEVINFO event is to be able to uniquely identify individual devices (e.g. for use with the fingerprint feature). For this purpose, it doesn't really matter if the serial number (as returned by libdivecomputer) doesn't match the real serial number (as printed on the hardware). That's why I didn't bother to decode it properly, and just return the raw bytes wrapped in a 32bit integer. In this sense the serial number is more like an opaque device id, and it was not really intended to be shown to the end-user (e.g. for warranty purposes).
Until now, all serial numbers did nicely fit into a 32bit integer, but if there ever happens to be a device where this isn't possible, we can always calculate some hash, just like you already do in subsurface. But of course that will only work if we make no guarantees that the libdivecomputer serial number will match the real serial number.
You are solving the wrong problem. As was discussed earlier on the list, being anle to show the "user readable" serial number in the dive log is a feature - one that we are interested in having. That's why Linus suggested a new callback that returns the serial number STRING for those devices where it is known how to create it. So no change is suggested to the existing 32bit number - just the addition of a human readable serial number to the API. /D
On 10-01-13 16:57, Dirk Hohndel wrote:
Jef Driesen <jefdriesen@telenet.be> writes:
It could be as simple as a new libdivecomputer callback ("given this serial *number*, what is the string for it?"), but it would probably be better to expand the current DC_EVENT_DEVINFO event to simply give us a string in addition to the number. Because it's not necessarily the case that all serial number strings can even be represented as a single 32-bit integer.
The primary purpose of the serial number in the DC_EVENT_DEVINFO event is to be able to uniquely identify individual devices (e.g. for use with the fingerprint feature). For this purpose, it doesn't really matter if the serial number (as returned by libdivecomputer) doesn't match the real serial number (as printed on the hardware). That's why I didn't bother to decode it properly, and just return the raw bytes wrapped in a 32bit integer. In this sense the serial number is more like an opaque device id, and it was not really intended to be shown to the end-user (e.g. for warranty purposes).
Until now, all serial numbers did nicely fit into a 32bit integer, but if there ever happens to be a device where this isn't possible, we can always calculate some hash, just like you already do in subsurface. But of course that will only work if we make no guarantees that the libdivecomputer serial number will match the real serial number.
You are solving the wrong problem. As was discussed earlier on the list, being anle to show the "user readable" serial number in the dive log is a feature - one that we are interested in having.
That's why Linus suggested a new callback that returns the serial number STRING for those devices where it is known how to create it. So no change is suggested to the existing 32bit number - just the addition of a human readable serial number to the API.
Oh, but I was mainly responding to the Linus' statement that the libdivecomputer serial number is wrong. But it's not really wrong if you take into account how it was supposed to be used. Yeah, I know, some documentation would have been nice here :-) Anyway, we can just fix the decoding of the serial number of course. For the fingerprint feature it doesn't have to be decoded correctly, but that does mean we can't decode it correctly either (see attached patch). I would prefer that instead of adding another callback with a string. A plain number is more convenient than a string (I hate string processing), especially for things that are numbers. How the number is stored by the device is irrelevant. If we ever encounter a serial number that isn't a number or is larger than 32 bits, than we can always reconsider. Jef
On Jan 12, 2013, at 1:15 PM, Jef Driesen wrote:
On 10-01-13 16:57, Dirk Hohndel wrote:
Jef Driesen <jefdriesen@telenet.be> writes:
It could be as simple as a new libdivecomputer callback ("given this serial *number*, what is the string for it?"), but it would probably be better to expand the current DC_EVENT_DEVINFO event to simply give us a string in addition to the number. Because it's not necessarily the case that all serial number strings can even be represented as a single 32-bit integer.
The primary purpose of the serial number in the DC_EVENT_DEVINFO event is to be able to uniquely identify individual devices (e.g. for use with the fingerprint feature). For this purpose, it doesn't really matter if the serial number (as returned by libdivecomputer) doesn't match the real serial number (as printed on the hardware). That's why I didn't bother to decode it properly, and just return the raw bytes wrapped in a 32bit integer. In this sense the serial number is more like an opaque device id, and it was not really intended to be shown to the end-user (e.g. for warranty purposes).
Until now, all serial numbers did nicely fit into a 32bit integer, but if there ever happens to be a device where this isn't possible, we can always calculate some hash, just like you already do in subsurface. But of course that will only work if we make no guarantees that the libdivecomputer serial number will match the real serial number.
You are solving the wrong problem. As was discussed earlier on the list, being anle to show the "user readable" serial number in the dive log is a feature - one that we are interested in having.
That's why Linus suggested a new callback that returns the serial number STRING for those devices where it is known how to create it. So no change is suggested to the existing 32bit number - just the addition of a human readable serial number to the API.
Oh, but I was mainly responding to the Linus' statement that the libdivecomputer serial number is wrong. But it's not really wrong if you take into account how it was supposed to be used. Yeah, I know, some documentation would have been nice here :-)
But that will be all better with the fully documented next API :-)
Anyway, we can just fix the decoding of the serial number of course. For the fingerprint feature it doesn't have to be decoded correctly, but that does mean we can't decode it correctly either (see attached patch). I would prefer that instead of adding another callback with a string. A plain number is more convenient than a string (I hate string processing), especially for things that are numbers. How the number is stored by the device is irrelevant. If we ever encounter a serial number that isn't a number or is larger than 32 bits, than we can always reconsider.
Well this works, but I really would LIKE a string. I'll tell you why. Remember the Atomics documentation where they tell us "it's these four digits, then a dash, then those four digits". I bet there are other vendors that have letters in there, too. So having libdivecomputer give us a \0 terminated string would really be lovely. Can I ask everyone on both of these mailing lists to take a look at their actual divecomputers and tell us what format your serial numbers have, please? /D
On Sat, Jan 12, 2013 at 1:36 PM, Dirk Hohndel <dirk@hohndel.org> wrote:
Well this works, but I really would LIKE a string.
Absolutely. The serial number *has* to be a string. Serial numbers really aren't numbers. The "identification number" (the thing that libdivecomputer now calls the serial number) can be a 32-bit number. That always works for identification, even if for some dive computers it might then have to be some hash of the "real" identification mark. But serial numbers simply aren't numbers. They have dashes, they have letters, they have specific formats. Even when they are "numbers" they may have very specific limits ("6 decimal digits zero-padded" or whatever) So despite being called "numbers", they are very much strings. They CAN NOT be represented as a numeric entity. Linus
On 12/01/13 21:36, Dirk Hohndel wrote:
Can I ask everyone on both of these mailing lists to take a look at their actual divecomputers and tell us what format your serial numbers have, please? /D
Mares Nemo Wide 2 : 6 digit numeric
On Sat, Jan 12, 2013 at 1:47 PM, Tim Wootton <tim@tee-jay.demon.co.uk> wrote:
Mares Nemo Wide 2 : 6 digit numeric
The shearwater predator seems to be a 8-digit hex number judging by their manual. The atomic aquatics seems to be two 4-digit decimal numbers with a dash in between. Judging by some recalls on the net, some Oceanics have 4-digit numbers, some have six, and they seem to use hex numbers at least for their firmware release, but the serial numbers seem to be decimal. Anyway, even when they are "numbers" it's clearly not possible to know how to format them. Hex or decimal? How many digits? Dashes? Linus
On Jan 12, 2013, at 2:33 PM, Linus Torvalds wrote:
On Sat, Jan 12, 2013 at 1:47 PM, Tim Wootton <tim@tee-jay.demon.co.uk> wrote:
Mares Nemo Wide 2 : 6 digit numeric
The shearwater predator seems to be a 8-digit hex number judging by their manual.
The atomic aquatics seems to be two 4-digit decimal numbers with a dash in between.
Judging by some recalls on the net, some Oceanics have 4-digit numbers, some have six, and they seem to use hex numbers at least for their firmware release, but the serial numbers seem to be decimal.
Looking at http://www.oceanicaus.com.au/page.php?id=29 it seems that most current Oceanics / Aeris / Hollis computers should have <letter><letter><dash><six numbers> as format.
Anyway, even when they are "numbers" it's clearly not possible to know how to format them. Hex or decimal? How many digits? Dashes?
Yes. Definitely. Back the backend might be able to figure this out for some computers. So your idea of having a callback that returns the string if we know how to create it is a good one. /D
On Sat, Jan 12, 2013 at 1:15 PM, Jef Driesen <jefdriesen@telenet.be> wrote:
Anyway, we can just fix the decoding of the serial number of course. For the fingerprint feature it doesn't have to be decoded correctly, but that does mean we can't decode it correctly either (see attached patch). I would prefer that instead of adding another callback with a string
I agree. I think there are two independent valid uses for the serial number: the "identify computer uniquely" one (that the current interface gives us and is fine at - and as you say, a single number is easier than a string), and the "try to give users a 'string' identifier for any paperwork issues (ie warranty registration etc)". So having a separate callback - and only for dive computers where we *can* compute that string - would be the right thing, I think. There is in fact advantages to the existing number that is only a unique identifier: it may be that for some dive computers you can't give the "real" serial number (because it may not even be programmatically available - maybe it's just physically stamped somewhere on the back, or just in the paperwork that came with the equipment). So changing the existing interface is likely a bad idea, but giving a *new* itnerface for the information if it is available would be nice. Linus
On Sat, Jan 12, 2013 at 9:15 PM, Jef Driesen <jefdriesen@telenet.be> wrote: [...]
Anyway, we can just fix the decoding of the serial number of course. For the fingerprint feature it doesn't have to be decoded correctly, but that does mean we can't decode it correctly either (see attached patch). I would prefer that instead of adding another callback with a string. A plain number is more convenient than a string (I hate string processing), especially for things that are numbers. How the number is stored by the device is irrelevant. If we ever encounter a serial number that isn't a number or is larger than 32 bits, than we can always reconsider.
We deal with two things here - serial number value as reported by a device - delivered by libdc now - serial number representation - this one is missed IMHO, a function/callback returning nicely formatted string is quite good idea. And it would be great to have both (let's just not mistake them for device identifiers, see my other e-mail). Another example of serial numbering, below. Sensus Ultra reports serial number value as 16-bit unsigned int i.e. 1234, but it is formatted/represented as U-XXXXX (i.e. U-01234) on the device itself. Looking at the ReefNet's website[1], the Sensus Pro's serial number is reported as SP-XXXXX (i.e. SP-01234). Regards, w [1] http://reefnet.ca/products/sensus/tour/photo/
On Thu, Jan 10, 2013 at 8:51 AM, Jef Driesen <jefdriesen@telenet.be> wrote: [...]
The primary purpose of the serial number in the DC_EVENT_DEVINFO event is to be able to uniquely identify individual devices (e.g. for use with the fingerprint feature).
There is nothing, which prevents two dive computers from two different manufacturers, to have the same serial number, isn't it? Not sure how it works for fingerprint feature, but IMHO it is worth keeping above in mind before someone starts to use the serial as device unique identifier. [...] Regards, w
On Jan 12, 2013, at 3:42 PM, Artur Wroblewski wrote:
On Thu, Jan 10, 2013 at 8:51 AM, Jef Driesen <jefdriesen@telenet.be> wrote: [...]
The primary purpose of the serial number in the DC_EVENT_DEVINFO event is to be able to uniquely identify individual devices (e.g. for use with the fingerprint feature).
There is nothing, which prevents two dive computers from two different manufacturers, to have the same serial number, isn't it?
Not sure how it works for fingerprint feature, but IMHO it is worth keeping above in mind before someone starts to use the serial as device unique identifier.
Absolutely correct. So Subsurface uses the tuple <model,serial> as unique identifier. /D
On 2013-01-13 01:01, Dirk Hohndel wrote:
On Jan 12, 2013, at 3:42 PM, Artur Wroblewski wrote:
On Thu, Jan 10, 2013 at 8:51 AM, Jef Driesen <jefdriesen@telenet.be> wrote: [...]
The primary purpose of the serial number in the DC_EVENT_DEVINFO event is to be able to uniquely identify individual devices (e.g. for use with the fingerprint feature).
There is nothing, which prevents two dive computers from two different manufacturers, to have the same serial number, isn't it?
Not sure how it works for fingerprint feature, but IMHO it is worth keeping above in mind before someone starts to use the serial as device unique identifier.
Absolutely correct. So Subsurface uses the tuple <model,serial> as unique identifier.
As long as that "model" includes not only the model number (as reported by the DC_EVENT_DEVINFO event), but also the family type, then everything is fine. The model number isn't guaranteed to be unique across different families (from the same or a different vendor) either. For example in the case of the "Suunto Vyper", the model number only gives you the "Vyper" part, while the family type will give you the "Suunto" part. So you need both. I'm aware subsurface does this correctly, but since this conversation is being cross-posted to the libdivecomputer mailinglist, this info applies to other applications too. BTW, there are some models too, where we can't really tell the difference between certain models. That's either because there is no model number available, or we just don't know yet where it's stored. An example of this are the Mares M1, M2 and Darwin. Jef
participants (5)
-
Artur Wroblewski -
Dirk Hohndel -
Jef Driesen -
Linus Torvalds -
Tim Wootton