<azonenberg>
mandl, dannas, mubes: please coordinate among yourselves to avoid stepping on toes or wasting time duplicating work
<azonenberg>
This is a good situation to have, this is the first time we've had so many people trying to work on one driver together lol
<azonenberg>
I just want to make sure it doesn't devolve into chaos
<d1b2>
<dannas> @azonenberg Oh. Would be great if he's managed to finish the whole thing. I've definitely been dragging my feet on this one. 🙂 I'll take a look at what he/she has done.
<mandl>
Hello team. nice to meet you are
<d1b2>
<dannas> @mandl Hello!
<mandl>
Hi dannas
<azonenberg>
mandl: (if you're new here this IRC is bridged to the 1bitsquared discord, d1b2 is the bridge bot)
<mandl>
noce to meet you
<mandl>
thanks for the starting code for 1104X-E
<d1b2>
<dannas> I guess i should have reached out to @mubes sooner.
<azonenberg>
anyway let me know when you've figured things out and delete one of the two PRs
<azonenberg>
So I have only one PR to review
<d1b2>
<dannas> @mandl Ok, so you've based your changes on my WIP stuff? I thougt it looked familiar. 🙂
<mandl>
yes i used our code ... and did some fixing ... now it works on my 1104X-E using LAN driver ..
<d1b2>
<dannas> @mandl You've added the missing timebase readings in AcquireData and you've implemented some (all?) missing trigger functionality.
<d1b2>
<dannas> Is you triggers working? I'll pull down your patch and try it on my scope.
<d1b2>
<dannas> Snowstorm outside and our snow showels are blowing away. I'll have to go outside for a bit. Be back in ten minutes or so.
<mandl>
ony the EDGE on... you can add and change it no more segfault ..
<d1b2>
<mubes> He he...it's nice to walk away from code and have it improve while you're not there 😁
<d1b2>
<dannas> @mubes Oh no, no no. That's certainly not what's happening here. Every added if-statement is an additonal maintaenace burden you know. 🙂
<azonenberg>
mubes: lol yeah i think the project is finally starting to reach that point of being useful enough that contributions are beginning to snowball
<azonenberg>
now if only someone would do that on the GUI side...
<d1b2>
<mubes> I think what you guys have done is great. My main concerns are in making it a bit more generic so we can accomodate other scopes in future without it becoming unmanageable spaghetti...so that's the main bit I want to take a look at.
* azonenberg
prays to the github gods for someone to send him a UI developer
<d1b2>
<dannas> The code originally just started out with me thinking I'd have to add a few small changes to account for minor diffs between the scopes.
<d1b2>
<dannas> @mubes One small step would be to extract the model specific code from AcquireData into model-specific functions.
<d1b2>
<mubes> Well, we're in direct contact with the coders at Siglent now so if you find bugs or problems please write them up and we can pass them along.
<d1b2>
<dannas> If it would have been just me I*d probably would have used switch statements instead of ifstatements
<d1b2>
<dannas> Maybe I'd even considered making specific sublcasses (but really not sure on the later one probably not)
<d1b2>
<mubes> Yes, deffo need to move to switch cos at the moment everything crashes into the SDS2000XP case
<d1b2>
<mubes> but that's OK, it's easy coding....just got to find the cycles.
<d1b2>
<dannas> And "if it was me" is a strange thing to say for someone who actually did add the if-statements in teh first case 🙂
<d1b2>
<dannas> Now it's great that you @mandl also are interrested in finishing the code. A small, small part of me felt a little disturbed by the fact that you didn't mention that the code was from me in the first place.
<d1b2>
<mubes> The timeout guards are probably model specific too. Can you try on your scope if you support multiple statements on the same line, separated with ';' ? If so you can reduce a lot of those configs to single statements for one round trip time.
<d1b2>
<dannas> But that's just my vanity and pride I guess 🙂 I'm really not sure if I wanna see the code attributed to me (drivers tend to be messy things)
<d1b2>
<mubes> (That doesn't work on the 2000XP properly...yet)
<d1b2>
<mubes> @dannas get your name in the comments, so we've got someone to blame 🙂
<d1b2>
<dannas> @mubes That's one thing I have been wondering about specifically: if the scope accepts multiple statements.
<azonenberg>
mubes: on that note, longer term
<azonenberg>
we may want to consider checking firmware rev returned by *IDN?
<azonenberg>
and refusing to run if the fw is too old
<azonenberg>
i.e. if we want to require some bug fix they have
<d1b2>
<mubes> If you've got a write-only command then you can finish it with *OPC? to know when it's done
<d1b2>
<dannas> @mubes. You mean using @OPC instead of timeouts?
<d1b2>
<mubes> @azonenberg Yes, we have to do that because some returns will vary between f/w version in future.
<d1b2>
<mubes> yes
<d1b2>
<dannas> +1 to adding firmware checks. I was surprised to see how many revisions has been released of the Siglent firmware.
<d1b2>
<mubes> They are very fixated on getting this stuff working.
<d1b2>
<dannas> Speaking of that: The latest (or one of the latest in case I've missed one) fw has support for add UI options in the scope to set labels. I wonder if that's exposed in the SCPI siglent command set.
<d1b2>
<mubes> The idea would be to be able to do something like :TIMEBASE:SCALE 1E-02;*OPC?
<d1b2>
<mubes> then you get a 1 back when it's done
<d1b2>
<mubes> ...and you can use converse rather than sendOnly
<d1b2>
<dannas> As for coordinating: What would be the step forward. 1. Should the code switch to switch statments (pun, heh) 2. In what state should the code be in before it's merged? 3. @mubes Anything else re the architecture of the driver that you think we better change now?
<d1b2>
<mubes> sendOnly has a 50mS guard....you might not need that for the 11xx
<d1b2>
<dannas> 50ms guard... I haven't really investigated how long time each transaction takes. Though I've noticed that some has had more lag than I wished for.
<azonenberg>
yeah we definitely are going to be pushing them for performance improvements as well
<d1b2>
<mubes> @dannas (1) yes, let's move to switch. I have to do a full integration test on your code on the 2000XP to make sure nothing got borked in the transition, and I'd prefer to not have to do that multiple times.
<azonenberg>
as far as setting labels, you mean like channel names?
<d1b2>
<dannas> It's a little complicated if @mandl and me both have PRs. Can we accept @mandl in a non-finished state just for easier coordination later on? To me that sounds suboptimal from a code history perspective. But it would simplify later cooperation.
<d1b2>
<mubes> (2) Don't care, if it runs its better than what was there. We can tidy as we go, as long as we all agree that we don't leave it in a mess (you're welcome to push to my repository if @azonenberg doesn't want scruffy WIP in main/master)
<azonenberg>
I'm fine with merging WIP as long as it works better than what's there
<d1b2>
<dannas> @azonenberg Yes, I meant setting labels for channels. I can do it in the scope UI but don't know how to do it via SCPI
<azonenberg>
my general rule is that any change to the code should result in functionality which is a strict superset of what was in there already
<azonenberg>
i.e. you can add new stuff that does nothing / doesn't work as long as nothing that used to work stops working
<d1b2>
<dannas> @azonenberg Definitely agree on that. Every commit should add value.
<azonenberg>
as we get closer to a release that will change but i wanted 1000 series support prior to v0.1
<d1b2>
<mubes> (3) The architecture of the driver is largely fixed by Andrews existing abstraction. The sendOnly/converse stuff is different to the other drivers but thats mostly the only real change. I noticed a few ? instead of if in the code which I greatly prefer as long as the condition and consequence is reasonably obvious since it reduces vertical height.
<d1b2>
<dannas> So if I, mubes and maybe someone else can run @mandls PR, then @azonenberg can just merge that one I'd say. I'm not really sure what changes that PR has done, so I'm trusting mandl that he found some bugs.
<d1b2>
<dannas> @mubes Oh no, don't head into code layout territory. 🙂 That never ends well.
<d1b2>
<mubes> If we can move the if's to switches then we can test....that kind of change can easily introduce 'issues'
<d1b2>
<dannas> Then... @mandl are you still there? Can you change to switch statements?
<mandl>
Yes i am online..
<d1b2>
<dannas> And sorry @mandl if I've been referring to you in third person. Internet communication is hard 🙂
<d1b2>
<mubes> ...or I can do it, just need to co-ordinate so we don't all try and do it.
<d1b2>
<dannas> @mubes I'm fine with pausing what I have and continue, once that PR is merged.
<d1b2>
<dannas> So @mandl or @mubes who be it to do the mindlessly boring task of rewriting many statements?
<d1b2>
<dannas> And probably face some nit-pick about code (that's the way things tend to go in PRs I think) 🙂
<d1b2>
<dannas> wonders if he should cut down on the emojis.
<d1b2>
<mubes> Well, it's @mandl's patch if he wants to have a punt at it, TZ dependent I guess
<d1b2>
<mubes> Then we can test the patch against 11xx and 2000XP, and commit it as a new working base.
<d1b2>
<dannas> Time to get the kids in bed. I'm back in 45min or so. @mubes Yup. merging that PR will simplify collaboration.
<d1b2>
<mubes> I'm off to cook dinner. Back later.
<d1b2>
<dannas> I'll investigate a bit tonight if my scope supports the OCP command and try out the dribver.
<d1b2>
<mubes> OK. @mandl are you able to pick up the switch conversion or shall I do it?
<mandl>
if(m_modelid == MODEL_SIGLENT_SDS1000)
<mandl>
change this to switch()...
<mandl>
??
<d1b2>
<mubes> yeah, with a yelp for the default case rather than defaulting to SDS2000XP.
<mandl>
i can do.. tommorow
<mandl>
yes default will SDS2000XP
<d1b2>
<mubes> OK. Keep an eye out then 'cos depending on what I get to do tonight I might take a look.
<d1b2>
<mubes> First report is promising. It connects to the 2000XP and streams/triggers correctly 😁
<mandl>
sound good
<d1b2>
<dannas> @mandl Weird. The first time I ran your branch things worked great. But after I restarted the scope and restarted glscopeclient I've had too times in a row where no waveforms for channels 2, 3, 4 are displayed. Will continue investigating. Great that the trigger is displayed but it seems like the scale for the trigger is not correct. If I move it just a little higher via glscopeclient it goes outside the range of the waveform and the
<d1b2>
triggering stops.. Will investigate that as well.
<d1b2>
<dannas> blushes for not recognizing that the header of the data matches the format that the existing ReadWaveformBlock expects
<d1b2>
<dannas> @mandl Did you change more things besides triggering and the parsing of the waveform data in AcquireData?
<d1b2>
<dannas> I don't consider any of the problems I listed above showstoppers for merging the mandl PR. And they might just as well be me doing something stupid.
<mandl>
@Dannes i did the most changes in AcquireData()
<mandl>
but set the Memory_SIZE to 7k
<d1b2>
<dannas> @mandl ok.
<mandl>
but this works not wehn scope is in Stop/Run
<mandl>
@Dannes my work is not perfect.... but we get data..
<mandl>
@Dannes we i also some "magic to understand"...:-)
<d1b2>
<dannas> @mandl Yes, we get data and that is great! The problems I'm seeing might very well be me doing something wrong here.
<d1b2>
<dannas> "magic to understand" What do you mean?
<mandl>
How to change memory size wehn scope is running
<d1b2>
<dannas> Doh, here's something not related to your PR, but I've senn these abort statements being triggered when I run glscopeclient on my Linux laptop.
<d1b2>
<dannas> @mandl I've also noticed that I can't change memory size when the scope is running. So far my working hypothesis is that it's a bug in the firmware.
<d1b2>
<dannas> @mandl I've found maybe five different behaviors which might be bugs in the siglent fw. Whenever I've mentioned one of them @azonenberg has asked me to give more detail so that he can forward a bug report to siglent.
<mandl>
workaround... stop trigger. press Auto on scope... open the dialog... change value ..
<d1b2>
<dannas> And I've replied several times that I've written down notes and will make a final report to him. Though, right now I can't find that text file. 😕
<d1b2>
<dannas> @mubes You asked me whether sds1104x-e supports multiple commands separated by semicolon and ending with *OPC?.
<d1b2>
<dannas> Yes appears to be the answer. I can set multiple settings and get a 1 back when the operation has finished.
<d1b2>
<dannas> Doh. Time's up for today. I'll test the branch again once the switch conversion work has been done.
<d1b2>
<mubes> @dannas do check that the setting you've made has 'stuck' though.
<d1b2>
<mubes> If you have a bug report then the minimum set of steps to repeat it via a telnet session are the best form of bug report. You can send it to me if you like and I'll send it along to siglent.
<mandl>
good night
<d1b2>
<mubes> Night.
mandl has quit [Ping timeout: 240 seconds]
<d1b2>
<mubes> @azonenberg Did you have a prettifier config around somewhere? I have a vague memory you did, but I can't see it offhand and it's not referenced from the coding policy if so..
<d1b2>
<mubes> @dannas Here's an example of a typical bug report that seems to work well; Forcing a trigger while the scope is in single trigger mode causes it to stop responding to status requests, until the trigger mode is changed again; :TRIGGER:MODE SINGLE :TRIGGER:STATUS? Ready :TRIGGER:STOP :TRIGGER:STATUS? Ready :TRIGGER:MODE SINGLE :TRIGGER:STATUS? Ready (Push the 'single' key) :TRIGGER:STATUS? (No Response)
someone-else has quit [Quit: Connection closed]
bvernoux has quit [Read error: Connection reset by peer]