siglent-sds: Reimplement eseries acquisition logic and add new scope models

[voneiden]

Changes are implemented so that they should not affect other series besides eseries. These changes have been using SDS-1104X-E. Fixes broken support to all e-series scopes and adds new models: SDS1104X-U, SDS2202X-E, SDS2352X-E

The acquisition logic for the eseries has been completely reinvented, simplified and made quite error resistant. USBTMC support is included (although it is significantly slower than TCP). The acquisition logic for the eseries is also now completely isolated from older models, allowing maintenance and improvements to be made and tested without breaking legacy compatibility.

Addresses bugs:

• https://sigrok.org/bugzilla/show_bug.cgi?id=1355 (eseries)
• https://sigrok.org/bugzilla/show_bug.cgi?id=1356 (eseries)
• https://sigrok.org/bugzilla/show_bug.cgi?id=1242 (eseries)
• https://sigrok.org/bugzilla/show_bug.cgi?id=1628 (usbtmc/eseries)

[voneiden]

I've re-rebased this PR against the master since the original PR was already 8 months ago and in a state of merge conflict. At this point however I'm not able to comment if these changes work anymore as originally intended as some of my code from the other PR was partially cherry-picked without due care. Therefore I suspect the whole thing just segfaults. Or maybe the ESERIES works, but other siglent models segfault (see #114 (comment) )

The communication in sigrok community appears to be pretty abysmal these days and majority PR's just get ignored, so I'm not keen on putting any more hours into this.

[iommu]

This fixed sigrok with my sds1104x-u 👍🏼 thank you!

Only issue I had was a slight deviation in my scope's USB product ID (I'll push a pull request)

[Tylermarques]

@gsigh Sorry to tag you here, but I see you're an active maintainer and I was wondering if we could get this merged sometime soon. The original bug is here.
If I can do anything to help with the merge process please let me know

[tomba]

Jfyi, I tested this today (merged to the latest libsigrok master) with my SDS1204X-E, and it makes pulseview work.

[laf0rge]

I can also confirm that analog acquisition is working rather reliably with (a rebase of) this PR on my SDS1104X-E. This is contrary to master, which is unusable. Unfortuantely, digital acquisition seems not supported. I tried for quite some time to get any of the SLA1016 signals to work but then discovered related log messages.

I wonder when the maintainers are going to either merge this PR or provide meaningful feedback what should be changed to get it merged.

[voneiden]

Thanks for the reviews. There's an ongoing thread on the mailing list on the state of the project.

It seems like the sole active maintainer doesn't have (unsurprisingly) the capacity or interest to process PR's, but at the same time the project does not welcome new maintainers who could do that. So it's a deadlock for the unforeseeable future.

Gerhard said in the linked thread that ANYONE can do reviews/test, but what good does it do if there is no intention/interest to merge the PR in the first place I might ask.

And how does one even pull off testing this PR properly - as this this driver supports, what, 25 different Siglent oscilloscope models from different eras each with their quirks and protocol differences.

As far as I recall I proposed few years back that I could split the driver to reduce the testing burden but got no feedback

Anyway, probably shouldn't merge this anyway without an active maintainer willing to look after the siglent driver to ensure that new bugs introduced here get fixed in a timely manner.

[raphCode]

I just read along on the mailing list, and as far as I understood it, the "obstacle" to merging PR's are concerns about maintainability. I believe this includes quality of submitted code and commit messages (think git blame) as well as the question who tests or maintains the feature after it has been merged.

Gerhard said in the linked thread that ANYONE can do reviews/test, but what good does it do if there is no intention/interest to merge the PR in the first place I might ask.

I think his main point is that the community should engage more. This means that other people than Gerhard take a look at the code and give feedback etc. People with the hardware can test functionality and report what works or what not. Someone could come forward to maintain the code after it was merged.

That being said, I don't know if once all conditions are fulfilled, someone merges the code. But nonetheless, I see some PRs got merged in recent time, so maybe?

---

My try in this PR:
I spent time reviewing the code superficially, and it looks reasonable to me:
The old behavior (for other models) is still available, the commit only seems to adds code for the new models.
We have at least 4 people that confirmed this works for them, 3 here, one in the sigrok issue tracker.

Potential improvements:

• add documentation for the protocol used
• I see doxygen comments mentioned in the HACKING file, maybe they can be helpful understanding the new functions?
• are there tests that can be added for the new code paths?
• would you be able to maintain this in the future?
• makes splitting the driver sense? Even if you didn't receive answers from the maintainers, what do you think? Are there examples right now in the codebase for/against driver splitting?

[voneiden]

I think his main point is that the community should engage more.

While it's a reasonable thing to wish for, holding PR's as hostage to encourage/force users to contribute to free software is certainly an odd position to take. I'm not really convinced that that is the ulterior motive. But I'll refrain from speculating.

would you be able to maintain this in the future?

Sorry, given the state of things with the project organization, I prefer not to be involved.

makes splitting the driver sense?

Ideally there would be common/shared functionality and clearly isolated hardware specific functionality. The complete splitting of a driver is not ideal since it will duplicate code but I proposed it to remove the need to consider and test older hardware generations.

---

FYI if anyone wants to work on this you're free to use any code in this PR with or without attribution. But I highly recommend getting in touch with someone who has commit access and make sure your effort is welcome before you start pouring on your free time on this. Just ping in this PR so other Siglent users can find your work too.

Please do note that there is also a conflicting PR: #176

[raphCode]

While it's a reasonable thing to wish for, holding PR's as hostage to encourage/force users to contribute to free software is certainly an odd position to take. But I'll refrain from speculating.

In fact, you are speculating by assuming that PRs are ignored on purpose and "held as hostage".
While I understand your frustration of work going unappreciated, to me the whole situation looks like a communication problem.
Contributions as I see them:

• github is not the preferred place for communication or code feedback for all sigrok developers, mailing list or IRC seem to be the traditional channels (source, search for "IRC")
• there seems to be less intrinsic motivation to "thankfully" merge all code contributions than I have observed in other open source projects (source, search for "butchery", "gift")
• the previous point, together with repeated requests to merge something, leads to the statement that "the community should engage more"
• there do not seem to be clearly defined rules/standards when a submission will be accepted, or what community members can do to really help the developers:
  • posts on the mailing list mention reviews, but in what form are they helpful? Just commenting "I looked at the code, looks good to me" probably doesn't convince anyone
  • similarly, there are surely different standards to compare against, code that looks okay to me may be considered "butchery" by veteran C programmers

At this point, I believe the issue is not solely of technical nature but also has a social component involved that should be addressed. All sides usually have complex reasons and motivations for their behavior which each seem perfectly reasonable to them, the goal is to learn about the other's side story.

[voneiden]

In fact, you are speculating by assuming that PRs are ignored on purpose and "held as hostage".

OK, bad wording, the bottom line was that it's the users fault that PR's are not received/welcomed/merged.

I believe the issue is not solely of technical nature but also has a social component

The social structure of the project appears to have changed quite significantly over the past few years. Like Gerhard himself wrote, there are only a few developers left. The old lead seems to be gone and to me it looks like Gerhard seems to emerge as the de facto leader now. The old contribution guidelines etc. probably don't quite reflect this change yet.

Part of me doesn't quite want to throw in the towel yet. Should probably get in touch with Gerhard and ask how he sees this driver should be handled (for example splitting it?). Is anyone here interested in co-authoring future changes? I'm quite tight on free time I can allocate to programming so working together would be nice.