azonenberg changed the topic of #scopehal to: libscopehal, libscopeprotocols, and glscopeclient development and testing | https://github.com/glscopeclient/scopehal-apps | Logs: https://libera.irclog.whitequark.org/scopehal
Degi_ has joined #scopehal
Degi has quit [Ping timeout: 268 seconds]
Degi_ is now known as Degi
<d1b2> <david.lenfesty> How are you liking imgui as a toolkit in general?
<azonenberg> I quite like it. A few things like toolbars were a bit awkward as there is no first class support for them
<azonenberg> it's just a window with a bunch of image buttons in it and you ahve to stick it at the top of the window yourself, etc
<azonenberg> but it's excellent for debug tools and property dialogs and such
<azonenberg> which is most of what my gui needs are
<d1b2> <ehntoo> Finally got back to my GPD-3303S driver. It was pretty painless, and after working out that the manual had CC/CV states in the status bits backwards, most everything seems to work pretty well. https://github.com/ehntoo/scopehal-apps/tree/add-power-supply-feature-conditions https://github.com/ehntoo/scopehal/tree/gwinstek-gpdx303s
<d1b2> <ehntoo> I'll open a PR tomorrow once I've hammered on it a little more, and investigated baking in support for this series of power supply's built-in parallel/series tracking modes on channels 1 & 2. If I don't extend the PowerSupply interface to support it, I should probably at least think about defaulting it to the independent mode on boot or something.
<azonenberg> Tracking is not supported in the current API but that would be a good thing to add
<azonenberg> Also missing: soft start and overcurrent shutdown can be turned on/off
<azonenberg> but you cannot control ramp rate or fuse blow time
<azonenberg> (and i forget if we have APIs to query those as feature flags or not)
<azonenberg> also i want to make some small changes to the gui, in particular allowing you to expand/collapse current and voltage trend plots separately
<azonenberg> And is that running on osx?
<d1b2> <ehntoo> that would be useful. I did add a function call in my branch for soft start/overcurrent shutdown, too. 🙂
<d1b2> <ehntoo> it is!
<azonenberg> Awesome. Lain sounds like she's making good progress on the waveform render vulkan conversion
<azonenberg> i have no idea what the remaining blockers to getting glscopeclient working on mac are
<azonenberg> but it sounds like ngscopeclient is basically there already, just not feature complete
<azonenberg> so at this point i'm tempted to say we'll put minimal effort into fixing gtk specific portability issues
<azonenberg> if it works with the new renderer on mac, great
<d1b2> <ehntoo> It seems so! I haven't found any new breakage in ngscopeclient yet, apart from my earlier assertion that HiDPI support seemed to be working possibly being a bit optimistic - it's very usable on a HiDPI display, but the font rendering isn't quite right. I'll take a closer look at that some time.
<azonenberg> and if not, we'll go full steam ahead on the new UI
<azonenberg> wrt DPI, i will be adding preferences for font sizes and also doing testing at various DPI scales wrt mouse interactions and rendering on the actual waveform plots
<azonenberg> that is where i expect more breakage
<azonenberg> but i will be testing aggressively on it
<_whitenotifier-7> [scopehal-apps] azonenberg pushed 2 commits to master [+0/-0/±8] https://github.com/glscopeclient/scopehal-apps/compare/f94d5e8da6ae...a518545d8b00
<_whitenotifier-7> [scopehal-apps] azonenberg a31e916 - Initial timeline rendering
<_whitenotifier-7> [scopehal-apps] azonenberg a518545 - Implemented mouse wheel zooming in waveform areas
<_whitenotifier-7> [scopehal-apps] azonenberg pushed 1 commit to master [+0/-0/±2] https://github.com/glscopeclient/scopehal-apps/compare/a518545d8b00...d611cbae5768
<_whitenotifier-7> [scopehal-apps] azonenberg d611cba - Implemented timeline dragging
<azonenberg> ok X axis (other than zoom-to-fit) is now implemented in ngscopeclient
<azonenberg> next up is the Y axis
<azonenberg> if i can get that done tomorrow and lain's shader work proceeds as planned, by early next week we should get waveforms rendering in ngscopeclient
<azonenberg> still a bit more glue to do around triggering and such
<azonenberg> none of the toolbar trigger buttons do anything yet
josuah has quit [Ping timeout: 244 seconds]
bvernoux has joined #scopehal
<_whitenotifier-7> [scopehal] MugheesChohan synchronize pull request #689: Adding burst width measurement filter - https://github.com/glscopeclient/scopehal/pull/689
<bvernoux> On latest ngscopeclient buit with mingw64 and tested on Windows10 I have following error
<bvernoux> ngscopeclient.exe
<bvernoux> terminate called after throwing an instance of 'vk::ValidationFailedEXTError'
<bvernoux> what(): vk::Image::bindMemory: ErrorValidationFailedEXT
<bvernoux> Assertion failed: m_valid, file C:/VulkanSDK/1.3.224.1/Include/vulkan/vulkan.hpp, line 1281
<bvernoux> For information previous build done 2 days ago worked perfectly
<bvernoux> I'm testing with gbd now
<bvernoux> let's try with break abort
<bvernoux> for info yes I have fonts and icons directory with files inside where ngscopeclient is run
<bvernoux> so it appears when loading icon "C:\\msys64\\home\\bvern\\glscopeclient_release_glslang_static/icons/24x24/trigger-start.png
<bvernoux> I confirm this icon is present C:\msys64\home\bvern\glscopeclient_release_glslang_static\icons\24x24\trigger-start.png
<_whitenotifier-7> [scopehal-apps] bvernoux opened issue #498: ngscopeclient Mingw64/ Windows10 exception at start seems related to TextureManager::LoadTexture() - https://github.com/glscopeclient/scopehal-apps/issues/498
<d1b2> <Mughees> So, in my burst width filter I notice this problem. I have implemented a filter parameter "Idle Time". Its default value is 1ms. Using only default works fine. But when I change this on the fly from 1ms->10ms->1ms, the in the Referesh() method of my filter I get the following weird values of Idle Time 1000000000000 Idle Time 1000000000000 Idle Time 9999999827968 Idle Time 999999995904. This disturbs the calculations.
<d1b2> <Mughees> Notice that I have just reverted back to 1ms from 10ms, even then I do not get back correct value..
<d1b2> <Mughees> Am I missing omething
<d1b2> <Mughees> Screenshot from 2022-09-18 13-30-55
josuah has joined #scopehal
<_whitenotifier-7> [scopehal-apps] bvernoux edited issue #498: ngscopeclient Mingw64/ Windows10 exception at start seems related to TextureManager::LoadTexture() - https://github.com/glscopeclient/scopehal-apps/issues/498
<_whitenotifier-7> [scopehal-apps] bvernoux edited issue #498: ngscopeclient Mingw64/ Windows10 exception at start seems related to TextureManager::LoadTexture() - https://github.com/glscopeclient/scopehal-apps/issues/498
<_whitenotifier-7> [scopehal-apps] bvernoux edited issue #498: ngscopeclient Mingw64/ Windows10 exception at start seems related to TextureManager::LoadTexture() - https://github.com/glscopeclient/scopehal-apps/issues/498
<_whitenotifier-7> [scopehal-apps] bvernoux edited issue #498: ngscopeclient Mingw64/ Windows10 exception at start seems related to TextureManager::LoadTexture() - https://github.com/glscopeclient/scopehal-apps/issues/498
<_whitenotifier-7> [scopehal-apps] bvernoux edited issue #498: ngscopeclient Mingw64/ Windows10 exception at start seems related to TextureManager::LoadTexture()/vk::raii::Image::bindMemory() - https://github.com/glscopeclient/scopehal-apps/issues/498
<_whitenotifier-7> [scopehal-docs] MugheesChohan opened pull request #50: Adding Burst width Measurment Filter's documentation - https://github.com/glscopeclient/scopehal-docs/pull/50
<_whitenotifier-7> [scopehal-apps] bvernoux edited issue #498: ngscopeclient Mingw64/ Windows10 exception at start seems related to TextureManager::LoadTexture()/vk::raii::Image::bindMemory() - https://github.com/glscopeclient/scopehal-apps/issues/498
<_whitenotifier-7> [scopehal-apps] bvernoux edited issue #498: ngscopeclient Mingw64/ Windows10 exception at start seems related to TextureManager::LoadTexture()/vk::raii::Image::bindMemory() - https://github.com/glscopeclient/scopehal-apps/issues/498
<_whitenotifier-7> [scopehal-apps] bvernoux commented on issue #498: ngscopeclient Mingw64/ Windows10 exception at start seems related to TextureManager::LoadTexture()/vk::raii::Image::bindMemory() - https://github.com/glscopeclient/scopehal-apps/issues/498#issuecomment-1250253594
<_whitenotifier-7> [scopehal-apps] bvernoux edited a comment on issue #498: ngscopeclient Mingw64/ Windows10 exception at start seems related to TextureManager::LoadTexture()/vk::raii::Image::bindMemory() - https://github.com/glscopeclient/scopehal-apps/issues/498#issuecomment-1250253594
<_whitenotifier-7> [scopehal-apps] bvernoux edited a comment on issue #498: ngscopeclient Mingw64/ Windows10 exception at start seems related to TextureManager::LoadTexture()/vk::raii::Image::bindMemory() - https://github.com/glscopeclient/scopehal-apps/issues/498#issuecomment-1250253594
<_whitenotifier-7> [scopehal-apps] bvernoux commented on issue #498: ngscopeclient Mingw64/ Windows10 exception at start seems related to TextureManager::LoadTexture()/vk::raii::Image::bindMemory() - https://github.com/glscopeclient/scopehal-apps/issues/498#issuecomment-1250260946
<_whitenotifier-7> [scopehal-apps] bvernoux edited a comment on issue #498: ngscopeclient Mingw64/ Windows10 exception at start seems related to TextureManager::LoadTexture()/vk::raii::Image::bindMemory() - https://github.com/glscopeclient/scopehal-apps/issues/498#issuecomment-1250260946
<_whitenotifier-7> [scopehal-apps] bvernoux edited a comment on issue #498: ngscopeclient Mingw64/ Windows10 exception at start seems related to TextureManager::LoadTexture()/vk::raii::Image::bindMemory() - https://github.com/glscopeclient/scopehal-apps/issues/498#issuecomment-1250260946
<_whitenotifier-7> [scopehal-apps] bvernoux edited a comment on issue #498: ngscopeclient Mingw64/ Windows10 exception at start seems related to TextureManager::LoadTexture()/vk::raii::Image::bindMemory() - https://github.com/glscopeclient/scopehal-apps/issues/498#issuecomment-1250260946
<bvernoux> So VulkanSDK on MSYS2/MINGW64 is a big mess
<bvernoux> I'm pretty the issue I have opened is because of that mix
<bvernoux> we are lucky that so far Vulkan basic stuff was working with mingw64 as VulkanSDK is built for Visual Studio 2017 for Windows and is clearly not usable (especially the static lib) with mingw64
<bvernoux> So we will go nowhere with VulkanSDK with mingw64 until we rebuild all ourself which is a huge work (which was started with glslang ...)
<bvernoux> with mingw64 for Windows
<bvernoux> and use that lib and not the one from the SDK ...
<azonenberg> mughees: at an initial glance, that might be due to the filter properties dialog using float32 when parsing string input from the user
<azonenberg> We could probably improve on the situation by using float64. generally speaking i haven't been super worried about extreme dynamic range in parameters because we were dealing with 8/10/12 bit ADC data
<azonenberg> and fp32 has more than that range
<azonenberg> bvernoux: i havent lood in detail at your thread yet
<azonenberg> but first thought: is it due to windows defaulting to BGRA instead of RGBA?
<azonenberg> perhaps we need to convert the textures to BGRA on windows instead of RGBA?
<bvernoux> I think it could be a deeper issue related to VulkanSDK
<bvernoux> but yes could be also something like you say
<bvernoux> Since the start the mingw64 build is not clean for VulkanSDK
<bvernoux> as VulkanSDK is built with Visual Studio 2017
<bvernoux> and so in fact what we can use in that VulkanSDK is only the header and we shall not use any lib ...
<azonenberg> Have you tried using the vulkan validation layer to test?
<bvernoux> no
<azonenberg> in particular, run vkconfig and enable all of the debug options in the loader
<bvernoux> I have not checked that but yes it is in option to better understand what happen
<d1b2> <david.rysk> Consumer GPU support of float64 is subpar
<azonenberg> david.rysk: that's a non issue. i'm talking about fp64 temporaries when parsing strnig input from the end user
<bvernoux> anyway it will be great MacOS check you ngscopeclient too
<azonenberg> in particular, Mughees is having a parameter to a filter that represnets a fairly long duration in time, using int64 internally
<azonenberg> but when printed for display and converted from text user input back to an int64, the intermediate values are fp32
<azonenberg> which leads to loss of dynamic range
<azonenberg> We don't convert directly to int because we have to allow for fractional values, for example 1.2 ms
<azonenberg> but that has to be converted to some huge number of femtoseconds
<azonenberg> So perhaps we need a separate fixed point conversion path that parses before/after the decimal point as separate integers
<bvernoux> we will nee float128 soon ;)
<azonenberg> well so there's three paths forward
<azonenberg> 1) fp64
<azonenberg> 2) full integer path
<azonenberg> 3) ignore it and accept that the dynamic range is good enough for this application
<azonenberg> I don't know if this filter needs exact values or not
<azonenberg> And lol. We already have a limitation on max waveform duration of about 2ish hours ? i forget the exact number
<azonenberg> but it's +/- 2^63 femtoseconds
<azonenberg> ok so to recap: Mughees - let me know if exact values (better than float32 temporary's dynamic range) are adequate for your filter, and if not we'll have to discuss how to proceed
<azonenberg> bvernoux: test enabling VK_LAYER_KHRONOS_validation and see if that sheds any light on your problem
<azonenberg> and explore the RGBA-vs-BGRA path. If we have to change some texture format on windows that's fine
<azonenberg> in particular, try changing the texture creation to use VK_FORMAT_B8G8R8A8_SRGB instead of VK_FORMAT_R8G8B8A8_SRGB temporarily
<bvernoux> I do not understand why RGBA or BGRA will do an exception
<azonenberg> this will break things on linux so we can't merge it as-is without a platform check
<azonenberg> but that's my first guess of root cause
<bvernoux> it will bring wrong color
<azonenberg> i'm not talking about the pixel data
<bvernoux> but nothing can detect if you have swapped the color
<azonenberg> i'm talking about you telling the card what the texture packing is
<azonenberg> if the API only supports RGBA and you tell it the data is BGRA it might choke
<bvernoux> ha yes
<azonenberg> and vice versa
<azonenberg> So if you just change the format field, red/blue will indeed swap
<azonenberg> we'd have to also swizzle the pixels
<azonenberg> my question is does the error go away
<bvernoux> anyway the fact we are using mingw64 vulkan DLL is not good too
<bvernoux> as it clearly does not match the VulkanSDK
<bvernoux> we are lucky so far
<bvernoux> as the symbol are found
<bvernoux> I was trying to rebuild vulkan-loader from git source code but it is a real mess
<azonenberg> The long term plan is and continues to be a pure visual studio build ditching mingw
<bvernoux> all is defined to be built on Linux or Visual Studio x64
<azonenberg> and i am working as rapidly as possible on ngscopeclient to reach that point
<bvernoux> Yes for vulkan stuff it will be clearly a must have
<bvernoux> the good news is on glscopeclient we do not have any issues so far
<azonenberg> ngscopeclient so far seems to be more portable. people have experimentally built it on mac and windows already
<bvernoux> as it does not use advanced vulkan stuff I imagine
<azonenberg> well glscopeclient's vulkan acceleration is entirely headless compute
<azonenberg> even with the new renderer, it will transition to a GL texture
<azonenberg> so we wont be doing any platform/color specific stuff
<azonenberg> ngscopeclient on the other hand actually draws directly to the screen
<azonenberg> with full vulkan
<azonenberg> its not advanced, in fact almost everything we do is basic vulkan 1.0
<azonenberg> but gets into platform specifics of how textures/framebuffers are handled
<bvernoux> vkconfig.exe on Windows provide totally different settings than in your example
<bvernoux> ha ok after reset of old config I can access to Validation conf
<azonenberg> and yes the default validation settings are light to none
<azonenberg> I explicitly turn on everything during development other than the "best practices"
<azonenberg> and info-level messages which get quite verbose
<azonenberg> you can also make it throw a debug breakpoint when an alert message is generated
<azonenberg> handy for finding which vulkan call triggered the message
<bvernoux> ok new details from Vulkan Validation
<bvernoux> VUID-vkBindImageMemory-memory-01047(ERROR / SPEC): msgNum: -1558817633 - Validation Error: [ VUID-vkBindImageMemory-memory-01047 ] Object 0: handle = 0xa21a4e0000000030, type = VK_OBJECT_TYPE_DEVICE_MEMORY; | MessageID = 0xa316549f | vkBindImageMemory()(): MemoryRequirements->memoryTypeBits (0x102) for this object type are not compatible with the memory type (0x7) of VkDeviceMemory 0xa21a4e0000000030[]. The Vulkan spec states:
<bvernoux> memory must have been allocated using one of the memory types allowed in the memoryTypeBits member of the VkMemoryRequirements structure returned from a call to vkGetImageMemoryRequirements with image (https://vulkan.lunarg.com/doc/view/1.3.224.1/windows/1.3-extensions/vkspec.html#VUID-vkBindImageMemory-memory-01047)
<bvernoux> Objects: 1
<azonenberg> with the settings this high up it even looks for memory bounds errors, a la valrgind/asan, in shader memory
<bvernoux> [0] 0xa21a4e0000000030, type: 8, name: NULL
<azonenberg> ok so it looks like we are trying to allocate texture memory from a pool that the driver doesn't want to use for textures
<bvernoux> so it seems you are not using the right memory type
<bvernoux> or something like that ?
<bvernoux> So it seems not related to the mess between mingw64 & VulkanSDK lib ;)
<azonenberg> no it's likely driver/gpu specific not windows or mingw related. or rgba
<azonenberg> Can you upload a vulkaninfo report somewhere?
<bvernoux> especially I was suspecting vulkan-1.dll
<azonenberg> my code that decides what kind of memory to use for the texture is likely making the wrong choice somewhere
<azonenberg> nvidia drivers are apparently less picky
<bvernoux> vulkaninfoSDK.exe does not provide any details now
<bvernoux> I suspect it is because of validation ...
<azonenberg> maybe you'll have to turn off validation for it. or at least make it not throw breakpoints on errors
<azonenberg> anyway, i have a good handle on what's goign on
<azonenberg> let me file a ticket and i'll look into it
<_whitenotifier-7> [scopehal] azonenberg opened issue #690: VulkanInit: detect valid memory types for textures and only allocate textures from those types - https://github.com/glscopeclient/scopehal/issues/690
<_whitenotifier-7> [scopehal] azonenberg labeled issue #690: VulkanInit: detect valid memory types for textures and only allocate textures from those types - https://github.com/glscopeclient/scopehal/issues/690
<bvernoux> It works when I set Vulkan Layers Management => Layers Fully Controlled by the Vulkan Applications
<bvernoux> But when in Validation mode we cannot output those info it is quite strange
<_whitenotifier-7> [scopehal-apps] bvernoux deleted a comment on issue #498: ngscopeclient Mingw64/ Windows10 exception at start seems related to TextureManager::LoadTexture()/vk::raii::Image::bindMemory() - https://github.com/glscopeclient/scopehal-apps/issues/498#issuecomment-1250260946
<azonenberg> Weird. anyway i'll look into it
<_whitenotifier-7> [scopehal-apps] bvernoux commented on issue #498: ngscopeclient Mingw64/ Windows10 exception at start seems related to TextureManager::LoadTexture()/vk::raii::Image::bindMemory() - https://github.com/glscopeclient/scopehal-apps/issues/498#issuecomment-1250314341
<_whitenotifier-7> [scopehal-apps] bvernoux edited a comment on issue #498: ngscopeclient Mingw64/ Windows10 exception at start seems related to TextureManager::LoadTexture()/vk::raii::Image::bindMemory() - https://github.com/glscopeclient/scopehal-apps/issues/498#issuecomment-1250314341
<bvernoux> Vulkan Validation is clearly a must have to debug such type of stuff
<bvernoux> else you can loose weeks to debug such complex code ...
<_whitenotifier-7> [scopehal] bvernoux commented on issue #690: VulkanInit: detect valid memory types for textures and only allocate textures from those types - https://github.com/glscopeclient/scopehal/issues/690#issuecomment-1250315338
<_whitenotifier-7> [scopehal-apps] bvernoux edited a comment on issue #498: ngscopeclient Mingw64/ Windows10 exception at start seems related to TextureManager::LoadTexture()/vk::raii::Image::bindMemory() - https://github.com/glscopeclient/scopehal-apps/issues/498#issuecomment-1250314341
<azonenberg> yes you have no idea how much time i lost to opengl shader bugs that the vulkan validation layer wouldh ave caught in seconds
<d1b2> <Mughees> Well, the parameter specifies the time between two bursts...So I guess it doesn't need to be that much accurate....Le me try using floats...if that solves my problem then I think we are fine
<azonenberg> no dont chagne any thing to float
<azonenberg> keep the time as int64
<azonenberg> i'm saying, it is converted to float when parsing the string the user types
<azonenberg> and you lose significant digits that way because float only has ~9 significant digits in decimal
<d1b2> <Mughees> hmmmm
<azonenberg> So the question is, is that a problem?
<azonenberg> so far we've never had a filter where a parameter needed that level of accuracy
<d1b2> <Mughees> hmmm
<d1b2> <Mughees> I think we need ths then
<azonenberg> So can you summarize how the block works / what it's measuring?
<azonenberg> and what this parameter does?
<d1b2> <Mughees> I feel something fishy with even this float conversion from string.....first for 1ms it gives a number..then if i jut simply change it to 10 then back to 1ms...the value is still not consistent
<azonenberg> Let me look
<d1b2> <Mughees> here is the docmentation pull request
<azonenberg> So when you start up the filter, it's using the hard coded integer value
<azonenberg> which is exact
<azonenberg> if you change the value, doesnt matter what you change it to
<azonenberg> then type back 1ms
<azonenberg> now you are using "1ms" converted to float and then to int64
<d1b2> <Mughees> oh
<d1b2> <Mughees> got it
<azonenberg> Perhaps the correct fix is to add a new function to the Unit class for converting a string directly to int64
<azonenberg> without passing through float32
<d1b2> <Mughees> Yayy
<azonenberg> and then make the filter properties dialog use is
<azonenberg> it*
<azonenberg> the challenge is that it has to handle decimal points and fractional values
<azonenberg> like 1.2 ms
<d1b2> <Mughees> I am still becoming used to the source code...so i would ideally want someone else look at it
<azonenberg> and ok i misspoke. it looks like we actually use float64 for the intermediate conversion not float32
<azonenberg> Which had previously been sufficient accuracy
<d1b2> <Mughees> The parameter type enum says: enum ParameterTypes { TYPE_FLOAT, //32-bit floating point number TYPE_INT, //64-bit integer TYPE_BOOL, //boolean value TYPE_FILENAME, //file path TYPE_ENUM, //enumerated constant TYPE_STRING, //arbitrary string TYPE_8B10B_PATTERN //8B/10B pattern };
<azonenberg> Yes, i'm not talking about that
<azonenberg> TYPE_INT is indeed an int64
<azonenberg> The function at issue is Unit::ParseString()
<d1b2> <Mughees> ok....i was refereing to Unit::ParseString()
<azonenberg> which converts a text string, using a given unit, to a float64 value
<d1b2> <Mughees> sorry
<d1b2> <Mughees> TYPE_FLOAT
<azonenberg> Yes. TYPE_FLOAT parameters are float32
<d1b2> <Mughees> so the float64 is agin converted to float32?
<azonenberg> Your idle time parameter is TYPE_INt
<azonenberg> so we convert float64 to int64 in the properties dialog
<azonenberg> and the resulting rounding error is what you're seeing
<d1b2> <Mughees> ok..
<d1b2> <Mughees> So now whos gonna tak p this
<d1b2> <Mughees> take up*
<d1b2> <Mughees> issue
<azonenberg> Well I guess the first question is, how important is this rounding error? In particular, the delta between the parsed value and the theoretically exact value of 1 millisecond is 4096 fs
<azonenberg> or 4.096 ps
<azonenberg> Which is an error of 2.4E-8
<azonenberg> even on my 16 GHz oscilloscope, that's about 1/6 of a single sample
<d1b2> <louis> 4ps error sounds totally acceptable to me
<d1b2> <Mughees> yeah
<azonenberg> louis: and this isn't 4ps constant error
<azonenberg> it's 4ps per ms. when measuring smaller intervals the error is smaller
<d1b2> <louis> even better
<azonenberg> below ~1ns it will be exact
<azonenberg> sorry below ~1us
<azonenberg> and then after there you start losing low bits
<azonenberg> S\o if this level of error is within tolerance there's no reason to change code at all
<azonenberg> i'm more than willing to add a ParseString() method that does exact int64 conversions if and when we have a demonstrated need for it
<d1b2> <Mughees> can you think of an example difgtal signal where two bursts can be separated by a tie smaller than that?
<d1b2> <Mughees> pcie maybe or something like that
<azonenberg> mughees: PCIe gen5 is 32 Gbps which is 31.25 ps
<d1b2> <Mughees> i think not
<azonenberg> your rounding error is about 1/8 of a bit at pcie gen5 when measuring a 1ms long interval
<d1b2> <louis> I think for now as long as the filter ingests it as a int64 we can go ahead that way, and if in the future we decide we need to shave off the last error it's a seperate PR to change
<azonenberg> Correct. the filter is working with int64, as are all time intervals in existing filters
<azonenberg> it was done for exactly this reason
<azonenberg> you can get exact values in code if you need them
<azonenberg> or we can rework the gui iff there is a demonstrated need
<azonenberg> in the meantime, fp64 gets close enough for everything we've done
<azonenberg> but no changes outside the gui will be needed in either case
<azonenberg> Also to be clear, the idle time is the minimum right?
<azonenberg> so it's basically the dead time with no transitions before it declares a new burst to have started
<azonenberg> but longer idle times are totally OK
<d1b2> <Mughees> yeah
<azonenberg> it might be best to rephrase the docs to make that more clear then
<d1b2> <Mughees> ok
<azonenberg> "minimum idle time with no toggles before declaring start of a new burst"
<d1b2> <louis> Say, while we're here azonenberg, wanted to run a thought by you about pulse filters
<azonenberg> sure
<d1b2> <louis> (But also generally anything that can ingest either digital or analog waveforms, e.g. frequency filter)
<d1b2> <louis> Which is they currently auto-threshold at 50% analog signal
<azonenberg> Yes. the idea is that if you want a different threshold you can do a threshold filter yourself at a level of your choice
<azonenberg> then pass that in
<azonenberg> it would be slightly more efficient to have a threshold parameter in the filter itself, but so far that hasn't been a performance bottleneck large enough for me to bother coding it up
<d1b2> <louis> Yeah. That was my thinking
<d1b2> <louis> The feature-parity I'm aiming for is with Tek filters that all have an options dialog like
<d1b2> <louis> Intel Xeon Gold 5320
<d1b2> <louis> whoops
<d1b2> <louis> So I'm wondering, longer-term maybe, if this fits neatly into the thoughts we had at some point about automatically creating shim filters between something producing an analog and something consuming a digital waveform.
<d1b2> <louis> Not a concrete idea yet, just something to think about that it's nice UX to be able to go Analog waveform -> measurement -> burst width and then configure thresholding there w/o having to go back and create a seperate threshold filter and then another burst width filter
<d1b2> <louis> I'm imagining maybe pulse filters consuming digital, and then automatically creating a threshold filter we don't display by default when they are invoked on an analog waveform. Then there could be a context menu item or something that said "Edit Threshold..."
<azonenberg> Yes. automatic thresholding is on the wishlist for everything
<azonenberg> basically if a filter is only taking a digital value and you pass analog, it will infer a 50% threshold filter
<azonenberg> and then let you change the threshold in the filter graph if you want
<azonenberg> the argument for folding it into the filter is it saves memory BW vs having to write the thresholded value back to ram and read it back
<azonenberg> so integrating will be faster
<azonenberg> so perhaps we can start by creating shim filters and if we find that we do it often for specific blocks we can consider adding native analog input support to said filter
<d1b2> <louis> Well, the other option I was thinking about was some mixin/helper class that provides all the knobs you'd want to tune for thresholding and then exposes a set of GetDigital() GetEdges() etc
<d1b2> <louis> and mixing that into the freq filter, pulse filters, etc
<d1b2> <louis> That is conceptually less clean to me though.
<d1b2> <louis> I'm not convinced that's major though since the digi waveform should be sparse and so pretty small to interchange, no? Especially for pulse filters.
<azonenberg> Well right now the threshold filter outputs a uniform waveform
<azonenberg> one sample per sample in the input
<azonenberg> digital waveforms coming off an LA are sparsified
<azonenberg> but not off the filter
<miek> it'd be nice if it there were a way to show it on the input plot, so you could drag it around like the trigger level
<d1b2> <louis> Oh huh. Is there a reason we don't?
<d1b2> <louis> I had just assumed it did
<azonenberg> Because you can't parallelize sparsing. this is actually something i want to look into
<azonenberg> also, i want it to run on GPU
<azonenberg> with hysteresis and sparsing
<azonenberg> the challenge being you cannot do random access into a sparse output waveform
<d1b2> <louis> Why not? You can't parallelize the final vector assembly but you can split into N chunks and sparsify each in parallel
<azonenberg> Yes. but also hysteresis
<azonenberg> which depends on previous state
<azonenberg> it's a solvable problem but not a trivial one
<azonenberg> will likely require multiple passes
<azonenberg> but i think it's worth doing in order to reduce workload of downstream filters
<d1b2> <louis> Yes, that's a more interesting problem
<azonenberg> anyway, major backend architectural/optimization work is on hold for the next week or so as i'm still pushing hard on the new frontend
<d1b2> <louis> I wonder how useful it would be to have a mechanism for filters to indicate if the want sparese or dense input
<d1b2> <louis> so that a filter that would benefit more from O(1) access than from sparsity can indicate for the threshold filter not to bother
<azonenberg> I want to overhaul the input port configuration for filters in general
<d1b2> <louis> I guess it still has to handle both since it could be hooked right up to a LA
<azonenberg> to allow data driven descriptions of what kinds of input are legal
<azonenberg> as well as perhaps hints about how to maximize efficiency
<_whitenotifier-7> [scopehal] azonenberg pushed 1 commit to master [+0/-0/±4] https://github.com/glscopeclient/scopehal/compare/90f926eb849c...d0f59d29c68f
<_whitenotifier-7> [scopehal] azonenberg d0f59d2 - Refactoring: renamed physical device to reflect that we use it for more than just vkFFT now
<_whitenotifier-7> [scopehal-apps] azonenberg pushed 2 commits to master [+0/-0/±5] https://github.com/glscopeclient/scopehal-apps/compare/d611cbae5768...752fb64a1e80
<_whitenotifier-7> [scopehal-apps] azonenberg 6af0ed8 - Added initial skeleton code to block out space for WaveformArea Y axis.
<_whitenotifier-7> [scopehal-apps] azonenberg 752fb64 - TextureManager: ensure we allocate memory from a compatible type
<azonenberg> bvernoux: please test 752fb64 and let me know if it solves your problem
<azonenberg> miek: re dragging theshold levels, that's an interesting idea but the logistics of it - especially if you have multiple decode overlays on one signal - are nontrivial
<azonenberg> i'm open to idea if we can figure out a good UX
<azonenberg> anyway, i'm gonna do a few other things and then get on to reviewing those PRs from mughees
<azonenberg> and then try and get Y axes working
<azonenberg> (in ngscopeclient)
<_whitenotifier-7> [scopehal-docs] azonenberg closed pull request #50: Adding Burst Width Measurment Filter's documentation - https://github.com/glscopeclient/scopehal-docs/pull/50
<_whitenotifier-7> [scopehal-docs] azonenberg pushed 4 commits to master [+2/-0/±5] https://github.com/glscopeclient/scopehal-docs/compare/e1940cdf256d...5d314813e058
<_whitenotifier-7> [scopehal-docs] MugheesChohan 8f18228 - Adding documentation of Burst Width Filter
<_whitenotifier-7> [scopehal-docs] MugheesChohan 587fb78 - Adding example diagram and filter input/outputs description
<_whitenotifier-7> [scopehal-docs] MugheesChohan 35a4830 - Updating description of idle time parameter
<_whitenotifier-7> [scopehal-docs] azonenberg 5d31481 - Merge pull request #50 from MugheesChohan/BurstWidthDoc Adding Burst Width Measurment Filter's documentation
<_whitenotifier-7> [scopehal] azonenberg closed pull request #689: Adding burst width measurement filter - https://github.com/glscopeclient/scopehal/pull/689
<_whitenotifier-7> [scopehal] azonenberg pushed 4 commits to master [+4/-0/±8] https://github.com/glscopeclient/scopehal/compare/d0f59d29c68f...80b6c0e67fba
<_whitenotifier-7> [scopehal] MugheesChohan 20f7799 - Adding burst width measurement filter
<_whitenotifier-7> [scopehal] MugheesChohan 5d3a644 - Cosmetic change
<_whitenotifier-7> [scopehal] MugheesChohan 875ef4a - Modifying filter to add a zero burst sample only if no burst was found
<_whitenotifier-7> [scopehal] azonenberg 80b6c0e - Merge pull request #689 from MugheesChohan/BurstWidthMeasureFilter Adding burst width measurement filter
<_whitenotifier-7> [scopehal] ehntoo opened pull request #691: Add basic support for GW Instek GPD-(X)303S power supplies - https://github.com/glscopeclient/scopehal/pull/691
<_whitenotifier-7> [scopehal-apps] ehntoo opened pull request #499: ngscopeclient Power Supply Feature Detection - https://github.com/glscopeclient/scopehal-apps/pull/499
<_whitenotifier-7> [scopehal-apps] ehntoo edited pull request #499: ngscopeclient Power Supply Feature Detection - https://github.com/glscopeclient/scopehal-apps/pull/499
<_whitenotifier-7> [scopehal] ehntoo edited pull request #691: Add basic support for GW Instek GPD-(X)303S power supplies - https://github.com/glscopeclient/scopehal/pull/691
<azonenberg> ehntoo: let me have a look...
<d1b2> <ehntoo> No rush. Happy to change anything that isn't ideal, as well.
<azonenberg> Yeah. I expect we will be making a lot of changes to e.g. the power supply APIs over the coming few weeks/months as we add more instruments
<azonenberg> We should probably also start documenting power supply drivers in scopehal-docs
<azonenberg> and non-scope drivers in general
<azonenberg> So please prepare a PR against scopehal-docs for this driver. i'll work on adding the other non-scope drivers when time permits
<azonenberg> also, weird that the channel count is the *first* digit in the model
<azonenberg> usually it's series number *then* channel count
<azonenberg> your error message on 88 says 3/4 channel scopes, not supplies
<azonenberg> You do not need to lock the transport mutex when doing SendCommandQueuedWithReply(), interlocking is handled within the transport
<azonenberg> the only time explicit locking is required is if you are sending multiple commands that must execute in sequence without anything interrupting them
<azonenberg> for example, the R&S supply you probably based this driver on has a concept of the current channel, and all command reference that
<azonenberg> so setting the current channel and querying the voltage has to be interlocked to prevent someone else from changing the current channel in the middle of it
<d1b2> <ehntoo> Ahhh, yeah, that makes sense
<azonenberg> But if your SCPI commands include the channel number in them, then there's no reason to lock
<azonenberg> the other big place you see interlocking is if you have a command like downloading waveform data from a scope that returns a huge block of data you read in multiple calls
<azonenberg> but for write-only commands or 1:1 request-response, the transport layer handles it all for you
<azonenberg> You will see some older drivers using SendCommand() and ReadReply() separately. These do not interlock and manual mutexing is required for all of them
<azonenberg> but we're moving away from the low level API except in specialized situations in favor of this
<d1b2> <ehntoo> Cool, that makes sense. I'll clean up the locking (I think pretty much every instance in this driver can go), put together a docs PR, and fix up the error message braino. :⁠-⁠)
<azonenberg> Also In functions that don't use their argument, rather than a void cast to ignore the unused argument
<azonenberg> the convention is to comment out the variable name
<azonenberg> e.g. GetPowerChannelActive(int /*chan*/)
<azonenberg> this achieves the same result with one less line so it makes the code a bit more concise. and in a longer function with a nontrivial body, this makes it clear at a glance that the argument is unused
<azonenberg> vs having to scroll down to the void cast
<d1b2> <ehntoo> Oh right, this is C++. I'm so used to C, some of those differences are going to take some getting used to.
<azonenberg> Lol
<d1b2> <ehntoo> Will update. :⁠-⁠)
<azonenberg> Other coding style notes: curly braces go on their own line, not same line as the conditional. and for single line bodies can be omitted entirely
<azonenberg> although if you have comments or a multi line condition or otherwise you think it's more readable, feel free to keep the braces. that's not a hard requirement
<azonenberg> but if used, they go on their own line
<azonenberg> Architectural thought: Rather than making SupportsX() be pure virtuals, it might make sense to provide a default implementation in the base class returning false
<azonenberg> this way as the set of feature flags grows, we don't have to add "return false" stubs to a zillion drivers
<azonenberg> and also provide default no-op implementations of the get/set functions for the associated properties
<azonenberg> the idea being that a minimal driver for a no frills supply should not have a ton of stub functions
<azonenberg> then we can add implementations only to the drivers that have such features
<d1b2> <ehntoo> can do.
<azonenberg> Ok let me know when you've applied those changes and i'll take another look
<azonenberg> it seemed good otherwise, these should be quick fixes
<azonenberg> (have not looked at the scopehal-apps PR yet)
<d1b2> <ehntoo> Based on your comments so far I think the -apps PR shouldn't change too much apart from some brace style changes, but it's a pretty small set of changes anyway. I'll give a shout once I've got things updated.
<azonenberg> Yep
<d1b2> <ehntoo> Should I modify the power supply methods that have feature detection methods (SetSoftStartEnabled, IsSoftStartEnabled, etc) to be defaulted to no-ops or other appropriate default implementations in the PowerSupply base class rather than pure virtuals as well?
<azonenberg> Yes, i think that is a good idea
<azonenberg> the idea is, we should be able to add a new feature to the base class and not have to modify an existing driver
<azonenberg> unless the driver is gaining support for it
bvernoux has quit [Quit: Leaving]
<azonenberg> the only things we should have be pure virtuals are things that we want to enforce every derived class *must* override
<azonenberg> meaning, absolutely core functionality that makes no sense to have a default implementation
<_whitenotifier-7> [scopehal-apps] azonenberg pushed 3 commits to master [+0/-0/±7] https://github.com/glscopeclient/scopehal-apps/compare/752fb64a1e80...d0b9bd62be76
<_whitenotifier-7> [scopehal-apps] azonenberg 2203989 - VulkanWindow: fixed some issues around resizing
<_whitenotifier-7> [scopehal-apps] azonenberg d56c25f - Updated submodules with burst length filter
<_whitenotifier-7> [scopehal-apps] azonenberg d0b9bd6 - Draw grid in WaveformArea's
<d1b2> <azonenberg> @louis: how's it looking? just have to draw and add mouse interaction code for the Y axis scale
<d1b2> <azonenberg> and then i think the core of the GUI will be about where we need for minimum viable demo stage
<d1b2> <azonenberg> minus the actual waveform rendering of course
<d1b2> <azonenberg> Still have to do a ton of properties dialogs, make the trigger controls actually do something, etc
<d1b2> <azonenberg> but this is all vulkan rendering, no Cairo overlays like we had in the GTK frontend
<azonenberg> And still a lot of glue around channel management and ref counting to do things like disconnect from a scope if we stop displaying any data acquired from it (directly or indirectly)