On 5/1/21 3:41 am, Gerhard Sittig wrote:
On Mon, 2021-01-04 at 15:59 +1100, Peter Skarpetis wrote:
I have written a dmm-serial module for the Meterman 38XR Multimeter
and have attached the patch below. I have also created preliminary
wiki pages which will be updated in due time.
This is my first time contributing to this project so if you could
tell me how to proceed it would be much appreciated.
Ideally you'd push future submissions to a public git repo (any
of them will do, just pick one). That's preferred when you expect
feedback and improvement iterations. Sending mail attachments is
less desirable. Notify the regulars on IRC when you pushed
something. That's preferred according to the HACKING doc file.
Sorry about that, I read the document but forgot about the public git
repository. You should have emailed back and would have gladly put it up
on github.
For this iteration I've picked up your diff. But your MUA has the
habit of trying to be helpful and terribly corrupting the content
in the process. Attachments would be equally affected though to a
lesser degree ("From" mangling which breaks the application of
the patch, and requires manual intervention, too).
Had to do some style adjustment. Then got carried away, and
renamed a few identifiers to reduce the text line length. Have
eliminated some comments and strived for better awareness and
less redundancy during maintenance. Hope I did not overdo it and
you still recognize your implementation.
I checked out your changes and I now understand the style expected by
the sigrok project. My next submission should be a lot cleaner.
Could you test if the version at
https://repo.or.cz/libsigrok/gsi.git/shortlog/refs/heads/wip/mm38xr-intro
git://repo.or.cz/libsigrok/gsi.git wip/mm38xr-intro
still works for you? If it does I can take this into mainline.
Aside from style and data type nits which hopefully improve the
readability and future maintenance of the driver, there was one
omission in the packet validity check (checked the frame
separator and "every first byte in the packet" :-] ).
Good catch of the array index in the packet validity check. Can't see
how i missed that. It was the first function I wrote.
I have tested your changes and can verify that the code still works.
One more change. In src/libsigrok-internal.h struct mm38xr_info should
be changed to
struct mm38xr_info { int dummy; };
I added all those fields at the start of development but in the end
wrote a stateless implementation of the driver and all those fields were
not required.
Either you can do it or I will do it once you commit the changes to
mainline.
Of course you get credited as the author who did the work of
creating the driver. If something was broken, that's my fault and
can get fixed. Just provide a followup commit. Thank you for
contributing the driver, and for updating the wiki. That's very
much appreciated.
The sigrok community is very welcoming and I look forward to more
contributions in the future.
Regards,
Peter Skarpetis
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
_______________________________________________
sigrok-devel mailing list
sigrok-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/sigrok-devel
