Hello Gerhard,
hello Kevin,

comments are in-line.

Best regards,
Helge

Am 10.03.2021 um 14:15 schrieb Kevin Grant:
>
> Hello Gerhard,
>
> On 2021-03-08 18:25, Gerhard Sittig wrote:
>
>> Is the basic operation fundamentally broken, or are just not all
>> of the product's features available while the device _can_ be
>> used to get captures and inspect or process the waveforms? In the
>> former case I'd agree to "not supported", in the latter I would
>> not.
>
> It appears to work at least partially for some users.
> Input threshold adjustment uses the wrong FPGA registers and wrong
> algorithm for threshold voltage DAC setting.
> Trigger setups use the wrong FPGA registers (won't work).
> Sampling setups also use the wrong FPGA registers.
> So, quite broken IMHO.
> This could be the result of the OEM changing the FPGA bitstream,
> perhaps deliberately to scupper sigrok support. Or just some mistakes.
>
Yes, the basic operation is fundamentally broken. The implementation
uses a construct of packed structs to layout USB reports. The packed
attribute is suppressed for some reasons in the MXE cross compile
configuration for the whole libsigrok project. This results in USB
reports for LA2016 with wrong content and length. The affected USB
reports are used to send essential parameters to the device. Therefore
the basic operation is fundamentalliy broken for at least one of the
supported platforms. There are some Git repositories around with
bugfixes including yours, Kevin's and mine. This is a good development,
but it's not available to the user who downloads an official version or
a nightly build.

So the Kingst-LA2016 is not supported for at least one platform. I am
not sure what the status at https://sigrok.org/wiki/Kingst_LA2016 should
express. If it means "works for at least one platform" it is supported.
Since sigrok is a cross-platform project it should mean "works for all
platforms that you find at https://sigrok.org/wiki/Downloads. If the
second meaning is intended, the current information is wrong. The next
girl that buys that device to use it with sigrok on Windows would be
disappointed.

>> And it would be nice if you'd spell the project's name correctly.
>
> Big 'S' is a big no. Got it.
>
>> That's when I had to
>> assume that it's just another echo of Helge's interesting idea
>> that the device would not be supported at all because its driver
>> hasn't shipped yet with a release. Or that his platform would not
>> be supported by design while the reason was mere non-portable
>> programming. Which can be fixed, just needs to be done. Properly.
>
> I have tested the patch from Helge and it fixes the byte packing for
> windows.
> Coupled with some other changes from me, we should have fully
> functional support for LA2016 and LA1016.
>
>> If you (Kevin, Helge, and maybe others) strongly
>> feel that the device page should discuss the
>> level of support, then I invite you to do put
>> the information there.
May I ask for the right to edit the Wiki? I would like to add a short
note about a platform. I would change the status to "development" and a
prominent note that it already works for most OS.

>>> I sent an experimental patch to Helge K. and Ray M. a week or
>>> so ago but haven't had any feedback yet.
>>
>> Coincidentally last week I tried to address those unresolved
>> feedback issues which Helge would not address, and nobody else
>> stepped up to make it happen or help make it happen.
>
> Helge is eager to help.
> Perhaps in some cases your requests are not specific or clear enough
> to be understood and acted upon.
>
>> Unfortunately I don't have access to the hardware, so I
>> could not test anything that I wrote. Would be nice if
>> you and others could check this version, which then
>> could go mainline when it's operational.
Yes, I can repeat what I send in the IRC chat where you asked. It works
for mey everything in your repository. I proposed to use the latest
version you prepared.

>> The driver source code suffers from a few more issues (format
>> strings, diagnostics style, choice of data types, unfortunate
>> dependencies between distant code paths
>
> I am an electronics engineer and am contributing to improve sigrok
> support for a device I find useful.
> Writing firmware or software is not my speciality but I 'get by', and
> I suspect
> that could be true for many potential contributors to sigrok.
> My functional patches may not meet the standard for submission.
> With some guidance I will get there I'm sure.
>
I also tried to match the coding sytle rules, and I thought I did (in
the second pull request). You changed a lot of formatting after my
changes, especially for code that was in the repository before. I had my
focus to the functional changes and the functions where they are applied.

Since you find a lot of code to reformat, I think it would be a good
idea to add the additional rules to the HACKING file in the reposiory
root. So contributors can find it. This could improve the code from
contributors and would reduce the effort to apply the changes.

>> magic incantations due to lack of a protocol spec
>
> The protocol spec for a proprietary device is a workaround, and a
> workaround should not be confused with a solution :-)
> Rather than spending time on a protocol spec I just wrote open
> firmware for the FX2.
>
> https://bitbucket.org/magellanic-clouds/sigrok-firmware-klafw/src/master/
> <https://bitbucket.org/magellanic-clouds/sigrok-firmware-klafw/src/master/>
>
> The FPGA bitstream is still required for now but it's interface is
> much easier to document than a proprietary USB protocol.
>
> Regards,
>
> Kevin
>
>
>
> _______________________________________________
> sigrok-devel mailing list
> sigrok-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/sigrok-devel




_______________________________________________
sigrok-devel mailing list
sigrok-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/sigrok-devel

Reply via email to