first stab at Aeris A300CS support

[Jef Driesen]

Dirk,

We don't have to use the existing multipage support. I just wanted to 
check whether we can re-use the existing infrastructure before adding 
something completely new. If it turns out your caching is a better 
solution, then fine no problem.

I went through your patches and have a couple of comments:

  * You used global variables for the cache and its bitmap. That's simply 
not acceptable. Please move this into the device handle. (I suggest you 
use the name "bitmap" instead of "tracker". In the ostc backend we 
already used "bitmap" for the exact same purpose.) And then you can 
pre-allocate everything in the open function.

  * Remove the read_big_pages from the layout. The layout is intended for 
the logic in the shared oceanic_common_device_xxx functions. But didn't 
change anything there. You actually introduced a bug in the vtpro and 
veo250 backends, because their layouts were not updated with the new 
field. So instead add some "bigpage" field directly to the device 
handle.

  * There are some other devices like the OC1 that also support the newer 
B4 command. But they appear to use 128 byte packets (8 pages) instead of 
256 byte packets (16 pages). So I would prefer that BIGPAGESIZE isn't 
hardcoded. That will make it easier to adapt it to other devices if 
needed. For example by setting the "bigpage" field above to 0 to disable 
the bigpage feature, and to 8 (OC1) or 16 (A300CS). I don't think such a 
change really affects your caching logic.

  * Libdivecomputer uses unsigned integers everywhere, especially if bit 
manipulations are involved.

  * For the checksum function. Just a crazy idea, but you never know. 
Maybe it's not a 256 byte packet with a 2 byte checksum. But two times a 
128 byte packet with a 1 byte checksum? That means all your second 
halves shift one byte, and you would probably notice some problems 
during parsing, but you never know. The reason why I think this might be 
a possibility is that the OC1 seemed to use such 128 byte packets with a 
1 byte checksum. Most likely I'm just wrong here, but it's worth 
checking.

  * Setting DTR/RTS twice looks really odd. Are you sure this is really 
needed? Does the Aeris application does this too? (Note that due to how 
the win32 api works, DTR/RTS are automatically set when configuring the 
line settings, but there are also separate calls for DTR/RTS.) For other 
devices I noticed that setting DTR/RTS may require a small delay after 
opening the serial port. So maybe you are just not doing it at the right 
time. Try to move it after that 100ms sleep.

  * I don't see the different baudrate anywhere in the patches. Did you 
forgot to include that part?

  * In the aeris_a300cs_version string, mask the firmware version with 
"\0\0". Otherwise it will only work for one firmware version.

Can you send me a full memory dump so I can try your code? Preferable 
with a portmon capture too.

Jef

On 2014-09-26 06:12, Dirk Hohndel wrote:
> This implements fairly basic support for the A300CS
> 
> - adds a cached reading function that supports transparent caching of 
> 16
>   byte reads, implemented as 256 byte reads (I cannot get the A300CS to
>   respond to the B1 command, but with B8 I can read 256 bytes; so I
>   implemented a neat little caching layer that makes 16 byte reads
>   available to the upper layers while in fact reading the larger pages
>   that contain the 16 byte block and caching them for later use).
> - adds support for an oceanic_atom2 open function that gets passed the
>   model information, since the A300CS needs us to toggle DTR and RTS 
> twice
>   before it's willing to talk to us. The old oceanic_atom2 open 
> function
>   remains unchanged in order not to break apps that might be using it
>   directly (tsk, tsk, tsk, your're not supposed to...)
> - adds the necessary changes to the parser to deal with the different
>   locations for various information; it supports the basics: date, 
> time,
>   duration, max depth, gasmixes plus depth, temperature and sensor
>   pressure from the samples
> 
> Missing / issues:
> 
> - there is a TON more information available. Some I know where it is, 
> but
>   can't easily pass it back because of libdivecomputer architecture 
> issues
>   (e.g. the firmware version). Others are clearly there but I haven't
>   figured out where (e.g. the deco state).
> 
> - the 256 byte read uses a two byte crc and I haven't been able to 
> figure
>   out the algorithm. I'm sure it's obvious and I'm just being stupid, 
> but
>   for now the check is commented out
> 
> This has been tested with Linus' new computer which has all of six 
> dives
> on it - and those six dives are rather homogeneous. This needs a lot 
> more
> testing, obviously. But so far so good - it works well from Subsurface 
> :-)
> 
> /D
> 
> 
> PS: Jef, I looked at the multiblock support. It adds more complexity 
> for
>     no benefit - so I decided not to add this to my patches. The 
> reduced
>     number of calls to the read function really makes no difference
>     whatsoever, but the changes make the code more complex and (IMHO) 
> more
>     fragile. The caching read on the other hand is easy, straight 
> forward,
>     and reasonably obviously correct (famous last words).
> 
> _______________________________________________
> devel mailing list
> devel at libdivecomputer.org
> http://libdivecomputer.org/cgi-bin/mailman/listinfo/devel