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@libdivecomputer.org http://libdivecomputer.org/cgi-bin/mailman/listinfo/devel