On Sat, 2021-01-23 at 08:43 +1100, Peter Skarpetis wrote:
>
> I have recently added support for the MHINSTEK MHS-5200A function
> generator to libsigrok and have created a pull request for you to
> merge.
>
> https://github.com/sigrokproject/libsigrok/pull/113
>
> I have tested it with an MHS-5225A but should work with other models
> as well.
That's not mergable in its current form. Can you do some cleanup?
Introduce the files with the proper names from the start. Move
the device communication to protocol.c and keep api.c as they are
for all other drivers. Fixup style issues. Consider squashing the
parts because: From the project's perspective this is the
introduction of an initial implementation of support for this
device (the first working version which covers a basic set of
features or all those features which translate to sigrok
infrastructure). Doesn't matter how many steps it took to get
there during internal development at your place. Consider the
perspective of those who integrate your contribution. Unless the
commits' activities really are separate things from a maintenance
perspective.
Can you estimate how much the MHS5200A and the JDS6600 devices'
protocols differ. You stated they do. Is it "just a little" or
"totally so"? Am asking since there is the question whether one
driver could support several dialects or minor variations, or
whether separate drivers are required since differences are too
many or too severe.
(We can also discuss this in IRC when you are around there.
Results in a more fluent conversation than mail exchange.)
Who else has got devices of that MHS series and can test?
virtually yours
Gerhard Sittig
--
If you don't understand or are scared by any of the above
ask your parents or an adult to help you.
_______________________________________________
sigrok-devel mailing list
sigrok-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/sigrok-devel
