<azonenberg>
mubes: there's a clang format file in the repo iirc
<d1b2>
<mubes> yeah, I was sure I'd used one before 🙂
<d1b2>
<mubes> yup, .clang-format, forgot they hide it!
<d1b2>
<mubes> @mandl , @dannas Here's SiglentSCPIOscilloscope.cpp modified to use swtiches instead of ifs. I've superficially tested it on SDS2000X+ and it's fine, will give it a full test when you've got a candidate version. I haven't pushed this anywhere, I'll leave it to you guys to update the PR etc.
<d1b2>
<dannas> Great with that in place it should be easier to add things in the future. With explicit checks for each model and the default case emitting a warning.
<d1b2>
<dannas> @mubes re sending multiple config commands: I did check that the settings persisted, but it was just a superficial test. Maybe some commands will be implemented differently. Will experiment some more tonight.
<d1b2>
<dannas> @mandl Note that @mubes change does nothing in the default case. I saw that you updated your PR last night to use switch statements but that in that change the SDS2xxx/SDS5xxx is handled in the default clause. I prefer @mubes approach since it makes the code easier to inspect.
<d1b2>
<dannas> I hope I don't come across as disagreeable pointing out such things.
<d1b2>
<mubes> @dannas just keep your guard up to some events not 'sticking'...but if you can batch commands there are some circumstances it will speed things up a lot.
<d1b2>
<dannas> @mubes Yes I'll be on my toes. 🙂
<d1b2>
<mubes> The code feels 'cluttered' with all of these switches in it, but I don't expect more than the two cases across all of the scopes (possibly with small tweaks for individual cases) so on balance I think it's better than subclassing. The sample collection routines definitely need simplifying and breaking into parts though.
<d1b2>
<dannas> @mubes I agree that the sample collection needs to be factored and maybe some things will be simplified. And I too was balancing between suggesting subclassing vs switches. But switches seems cleanest right now atleast as it keep related code closer together. It's easier to inspect how different models differ for specific commands that way.
<d1b2>
<dannas> Time for dayjob. Board bringup on the menu. Will check back in tonight (swedish time)
<d1b2>
<mubes> Yeah, for now at least I prefer switches cos you can see how the 'other' case does things...but that might change as the code solidifies. Good luck!
massi has joined #scopehal
<d1b2>
<mubes> @mandl I'm confused what has ended up in the repository. Can you please clarify? We shouldn't be doing work twice over.
<azonenberg>
mubes: nothing has been merged to master yet. i'm waiting for consensus from you three WRT which PR to review and merge, as well as a report from you to confirm that you've tested and nothing on the 2000 series broke
GenTooMan has quit [Ping timeout: 240 seconds]
<d1b2>
<mubes> Ta. Will let you know.
someone-else has joined #scopehal
<d1b2>
<mubes> @dannas @mandl Here is a file with the changes from both versions merged. @mandl please review and test on your scope, and push to your PR when/if you're comfortable with it. We need to avoid duplicating effort since we all have limited time available;