Modernize Hid/Bulk Lists #13622

Swiftb0y · 2024-09-02T18:06:30Z

No description provided.

src/controllers/hid/hiddenylist.h

src/controllers/bulk/bulksupported.h

src/controllers/bulk/bulkcontroller.cpp

Swiftb0y · 2024-09-06T14:32:11Z

anything else? can I squash the fixups?

src/controllers/bulk/bulksupported.h

JoergAtGithub · 2024-09-06T18:23:40Z

LGTM now!

Swiftb0y · 2024-09-06T18:40:11Z

Thanks. waiting for @daschuer

src/controllers/bulk/bulkenumerator.cpp

daschuer

some more comments.

src/controllers/bulk/bulksupported.h

daschuer · 2024-09-08T20:19:14Z

src/controllers/bulk/bulksupported.h

+    std::uint16_t product_id;
+    std::uint8_t in_epaddr;
+    std::uint8_t out_epaddr;
+    std::uint8_t interface_number;


interface_number is int in libusb. Is that an issue?

where do you get that from? its uint8 in the relevant struct. https://github.com/libusb/libusb/blob/467b6a8896daea3d104958bf0887312c5d14d150/libusb/libusb.h#L749

strange, everywhere else in libusb.h it is an int.

You point to bInterfaceNumber which is a byte. Everywhere else it is used as int because of -1 for invalid.

right, so int16_t or std::optional<std::uint8_t> or int? for the int you'll have to battle it out with @JoergAtGithub because he requested it

The interface number along with 0 have been introduced in a bulk action here:

5e162f8

What does 0 mean? Why are all 0?

We have the option to revert the change here and carry on, there was no user visible issue with the old types, was it?

Not really. I can revert if that is seriously a condition to merge... But fixing user-visible issues is not really the point of a refactor, is it?

What does the 0 below actually mean? Reading the code and the comment, it could be an unused value what we may want to indicate with -1 or is it actually interface 0?

Its the default. 0 on most devices, but not all (thus the table).

Where is the term "tweak" coming from and why is it referenced as value in the structure and not as tweak?

Because those are the nonstandard values we actually want to tweak. I can also call it "quirk", which is the terminology the linux kernel uses for this sort of thing.

Why are all 0?

Because the only device that has a non-zero interface has not been added yet.
https://github.com/acolombier/mixxx/blob/881a482f2000bf888fd9599922129981b00ac8aa/src/controllers/bulk/bulksupported.h#L13-L18

It's just numbering of the interfaces. We currently just support bulk devices from the manufacturer Hercules. And Hercules always use the first interface for bulk and the second for audio. This order is completely up to the manufacturer.

At least untill all numbers are known we need here int(-1) for invalid. see my other comment.

@acolombier may be able to share some more info on why detach kernel driver code is used on windows even though it doesn't work and why its not used on linux even though its at least possible.

This turned out to be the least minimal snippet of code to work on both Mac and Windows.

We have the option to revert the change here and carry on, there was no user visible issue with the old types, was it?

Not sure why we would revert a non-breaking issue here, but that would only impact Windows and Mac ability to use the S4 Mk3, on which effort of getting it officially supported by Mixxx has faded away, so I guess it doesn't really matter.
It is quite clear that this change was the happy path MVP to get Windows and Mac running, as some user was asking support for Mac on Zulip S4 thread. But I also don't own either and don't have the interest/money/time to do so and work on something comprehensive here.

src/controllers/bulk/bulksupported.h

src/controllers/hid/hiddenylist.h

src/controllers/hid/hidenumerator.cpp

Swiftb0y · 2024-09-09T12:09:16Z

whoops. sorry misclick, didn't mean to delete your review comment.

daschuer

It looks like, we have discovered an issue ..

daschuer · 2024-09-10T21:08:16Z

src/controllers/bulk/bulksupported.h

+        {{0x06f8, 0xb105}, {0x82, 0x03, 0}}, // Hercules MP3e2
+        {{0x06f8, 0xb107}, {0x83, 0x03, 0}}, // Hercules Mk4
+        {{0x06f8, 0xb100}, {0x86, 0x06, 0}}, // Hercules Mk2
+        {{0x06f8, 0xb120}, {0x82, 0x03, 0}}, // Hercules MP3 LE / Glow


According to this post:
https://mixxx.discourse.group/t/ubuntu-mint-hercules-dj-control-glow-not-detected/16207
The in_epaddr of Hercules DJControl MP3 LE is 0x82 in interface 1.
So it looks like assuming 0 was wrong here.

The interface number is also there for Linux and the API documentation does not exclude the usage there:
But

libusb_kernel_driver_active(m_phandle, m_interfaceNumber) libusb_claim_interface(m_phandle, m_interfaceNumber)

This is skipped in our implementation for Linux.

However I can read here https://github.com/libusb/libusb/blob/467b6a8896daea3d104958bf0887312c5d14d150/libusb/core.c#L2072C4-L2072C51:
// This functionality is not available on Windows.

But I think it is intended to be always called and then checked for LIBUSB_ERROR_NOT_SUPPORTED

Conclusion: something is broken here.

I agree, but I'm doubtful there is much to fix if we don't know what and have no hardware to test.

@acolombier Do you have a device to test it?

My idea is that we need to remove all conditional code and just let libusb do the OS abstarction.

That would be ideal, yes.

This means also that we must use in the table above -1 for retaining the old behaviour that might be broken on windows, until we know the real interface number and enable the device on windows. What do you think?

Why is the situation on windows special?

I don't know, it looks like it was working on Linux without specifying the interface when it has been introduced.

I hope @acolombier can give some insight as he has enabled bulk support for windows here:
5e162f8

I have the S4 Mk3 to test, which has non-zero interface. I have add the interface number, which was missing in the previous implementation, so I needed a none value to exclude the other device on which I couldn't confirm the value.

// This functionality is not available on Windows.

Without libusb_kernel_driver_active and libusb_claim_interface, I was unable to use my device on Windows. Note I have tested on a VM so perhaps there is some different behaviour to expect?

Also, note that libusb_kernel_driver_active/libusb_claim_interface should also be used on Linux. The reason for me excluding it is that the original Linux-only implementation didn't include it, and as I was implementing for Mac & Windows , I changed only the logic on these platform, but it appears to be a global oversight

libusb_claim_interface() is the one that is available on windows and probably the required one in your case.

daschuer · 2024-09-10T21:46:47Z

src/controllers/bulk/bulksupported.h

-        {0, 0, 0, 0, 0}};
+constexpr static bulk_support_lookup bulk_supported[] = {
+        {{0x06f8, 0xb105}, {0x82, 0x03, 0}}, // Hercules MP3e2
+        {{0x06f8, 0xb107}, {0x83, 0x03, 0}}, // Hercules Mk4


Interestingly the MK4 is reported here at:
https://mixxx.discourse.group/t/hercules-mk4-hid-under-linux-and-1-11/13447/3
{0x83, 0x02, 0} and {0x83, 0x03, 5}
Can we have the endpoint address twice? Is that the same end point?

no idea. @JoergAtGithub may know.

Workaround: Add a TODO, and use -1 as interfac number.

Interestingly the MK4 is reported here at: https://mixxx.discourse.group/t/hercules-mk4-hid-under-linux-and-1-11/13447/3 {0x83, 0x02, 0} and {0x83, 0x03, 5} Can we have the endpoint address twice? Is that the same end point?

This is not the same USB endpoint, because only the address 0x83 is identical, but not the number of the endpoint (0x02 vs. 0x03).

Endpoints and Interfaces are fields on different USB protocol layers:

Endpoints are raw hardware addresses which the operating systems USB kernel driver uses to prioritize the data transports with guranteed latency (Isochronous or Interupt transport mode) and non real-time transports (Control or Bulk transport mode)

Interfaces/Pipes are the a protocol level higher and depend not only on the hardware, but also on configuration from the host software (drivers like power-management or user mode applications). You can assign a Endpoint only to one USB Interface, otherwise you could get conflicting access. But an interface can run in different alternative modes (e.g. a camera can run in realtime mode for video using the endpoint Isocronous transfer mode or in high-resolution photo mode using the same endpoint in Bulk transfer mode instead).

See: https://beyondlogic.org/usbnutshell/usb5.shtml

I am still confused, because we don't use endpoint number in our code only the address.
Just double checked but I don't see the double 0x83 anymore, instead I found a double 0x81 with:
Interface 1 Endpoint Address 0x81
Interface 3 Endpoint Address 0x81
Interface 4 Endpoint Address 0x81

This information is not unambiguous. If you talk about the transport layer, you need to specify the pair of Endpoint Address and Endpoint Number:

It is still puzzling form me, because we do not consider the Interface number with Linux at all but it seams to work with the enpoint adress only, even without an Endpoint number. libusb_claim_interface() is written in a way that we may claim multiple interfaces with one handel.

mixxx/src/controllers/bulk/bulkcontroller.cpp

Line 199 in 4df2508

m_pReader = new BulkReader(m_phandle, m_inEndpointAddr);

libusb_bulk_transfer

Here we use only the enpoint adress
https://github.com/mixxxdj/mixxx/blob/4df25086d1aa060dcc3b2793bc5be20c7380e0a1/src/controllers/bulk/bulkcontroller.cpp#L42C18-L42C38

The underlying questions is what is the interface number for -1 and will it work like before?
How does the driver know which endpoint shall be used if there are tw owith the same address.
Or are they the same listed in two interfaces?

I am still confused, because we don't use endpoint number in our code only the address.
Just double checked but I don't see the double 0x83 anymore, instead I found a double 0x81 with:
Interface 1 Endpoint Address 0x81
Interface 3 Endpoint Address 0x81
Interface 4 Endpoint Address 0x81

I double checked this, and I found out, that the Endpoint-Number is part of the Endpoint-Address:
Each Interfaces can have up to 16 Endpoints and the address is a single byte build as follows:

Bits 0..3b Endpoint Number.

Bits 4..6b Reserved. Set to Zero

Bits 7 Direction 0 = Out, 1 = In (Ignored for Control Endpoints)

See https://beyondlogic.org/usbnutshell/usb5.shtml for reference

src/controllers/bulk/bulksupported.h

daschuer · 2024-09-11T13:40:56Z

I can confirm everything is resolved, exept the situation around unknown interface numbers.

Swiftb0y · 2024-09-13T20:00:16Z

done, though I can't test for regressions easily.

daschuer · 2024-09-13T23:23:39Z

src/controllers/bulk/bulkcontroller.cpp

-        } else {
-            qCDebug(m_logBase) << "Claimed interface for" << getName();
-        }
+    if (int ret = libusb_claim_interface(m_phandle, m_interfaceNumber); ret < 0) {


Let's avoid this confusing style with an assignment in an if condition.

Why is this confusing? Its just a new syntax which makes sense here since I don't need ret outside the if-body
https://en.cppreference.com/w/cpp/language/if#if_statements_with_initializer

Because it break the flow when reading the code. You expect an if condition like everywhere else but you read the assignment, which is every where else in the line before. This reduces IMHO maintainability and lead to misunderstandings. The benefit of the tight scope is negligible here.

The benefit of the tight scope is negligible here.

I disagree actually because if the variable does not get its own scope the variables storing the return values either need different names or the one variable needs to be reassigned. The different names is confusing because that leads me to believe that we're actually interested in this value while reassigning looks weird because the added mutability makes the code hard to reason about.

You expect an if condition like everywhere else but you read the assignment, which is every where else in the line before.

Well you only expect the condition there because you're not used to there being something else but this is a well designed and documented feature so its reasonable to expect it to be usable. The same applies to other "new" features such as lambdas or trailing return types.

Let me reiterate that this very much distinct from the relying on return value of operator= (eg. if (int var = f()) because that is mixing an init-statement with a condition while the latter one is clearly defined.

This is just a case of "Was der Bauer nicht kennt, frisst er nicht".

I won't insist on using this style, but resisting to embrace new and encouraged features and styles is a common pattern in code review. This is neither rewarding nor productive.

I have considered this a bit and it turns out that this is a general code style question, we should discuss on Zulip.

We have a plenty of places where we may consider to tighten the scope of a local variable, because we can. On the other hand we don't care about them, because it will just introduce boiler plate and nesting, interrupting the flow when reading the code. This is an issue when you only skim over the code, and especially when the if condition is broken over two lines of code.

I have no interest to ban this new statement from the Mixxx code entirely, but let's use it deliberately, backed up by a rule in our style guide.

This is here an edge case because we have two return values that will end up in the same variable of temporary nature otherwise. For now this is a new style, that does not fit to the rest of Mixxx. I am open to decide in any direction, but let's do it consistently.

I will not block the PR on this point, but prefer reusing ret here without a scope.

https://mixxx.zulipchat.com/#narrow/stream/109171-development/topic/C.2B.2B17.20if.20with.20init-statement

daschuer · 2024-09-13T23:27:56Z

src/controllers/bulk/bulkcontroller.h

+    std::uint16_t m_productId;
+    std::uint8_t m_inEndpointAddr;
+    std::uint8_t m_outEndpointAddr;
+    std::uint8_t m_interfaceNumber;


This should become an int, to match the usbapi type and to allow us to retain the 2.5 behaviour for the devices where we don't know the interface number by setting it to -1.

std::optional<std::uint8_t> would be so much more appropriate though.

Yes, when starting on a plan surface.

Libusb has decided to use -1 for invalid, so we should IMHO not introduce another concept. The API will just work with -1 and has internal checks, no need for any has_value() checks, in our code.

Libusb has decided to use -1 for invalid,

I was not able to find that definition. Can you link it to me?

The API will just work with -1 and has internal checks, no need for any has_value() checks, in our code.

Well, if we want to preserve how it works currently though, we need to check for -1 anyways, so we might as well check for has_value. I'm not sure if passing -1 deliberately is a good choice because it'll only make it more complicated to then handle the error (because we need to then distinguish from an error that occurred because we passed in -1 or we actually did pass in a garbage value we didn't account for).

It is just used literally in libusb like here:
https://github.com/libusb/libusb/blob/467b6a8896daea3d104958bf0887312c5d14d150/libusb/descriptor.c#L221
and
https://github.com/libusb/libusb/blob/467b6a8896daea3d104958bf0887312c5d14d150/libusb/os/windows_winusb.c#L2927

The only related macro is USB_MAXINTERFACES

From what I could tell is that @DaSchauer wants to introduce a sentinel value (-1 currently) that determines whether we should claim the interface at all. At least the current code only claimed an interface when it was non-zero. Whether that is correct is dubious so I just implemented it to claim it unconditionally.

While I don't object to that outright (it worked until now apparently, does anybody know better what to?), I insist on using a typesafe sentinel (std::optional (specifically std::nullopt)).

Anyways, I think we should try the state as currently implemented. None of us has a hercules controller anyways so its not like we can easily verify that this breaks something.

It is impossible to transfer data to or from an USB device without specifiing the interface (there might be a default, but it must be specified somehow). This is mandatory by the standard.
But hardcoding the Endpoint Addresses is not in sense of the standard, as these should be read back from the descriptors in the device, taking into account the currently active device configuration and alternate mode of the interface .

Claiming interfaces has been introduced recently in this PR #13008.
The code must have been worked before at least on Linux without claiming the interfaces. I have read that the Kernel automatically detaches the driver and frees up the interface for the user space application without explicit claiming.

The libusb implementation is implemented in a way that a handle can claim one interface or more. Transferring data works with the endpoint address only, without specifying the interface. So I assume that the endpoint address alone is sufficient for transferring the data. Claiming the interface seems to be only an exclusive access thing.

Searching the forum has revealed that the interface number is not always 0.

It is impossible to transfer data to or from an USB device without specifying the interface

I am afraid this is wrong. My conclusion is that claiming the interface 0 here for all devices is also wrong, since we don't have the knowledge which interface to claim. As a workaround we may restore the pre #13008 by not claiming the interface indicated by -1 or std::nullopt if you wish.
So we may call the variable:
kClaimNoInterface = -1;
or
kInterfaceUnknown = -1;

I agree, if we don't know the correct interface number, we shouldn't assume that the device use 0.

Swiftb0y · 2024-09-18T10:01:54Z

changed it so we only claim the interface if we know it. see the fixup

daschuer · 2024-09-18T14:50:57Z

src/controllers/bulk/bulkcontroller.cpp

+                                 << ":" << libusb_error_name(error);
+            libusb_close(m_phandle);
+            return -1;
+        }
    } else {


This needs to be moved to the inner scope

whoops of course. fixed

whoops, of course. done

daschuer · 2024-09-18T14:58:05Z

src/controllers/bulk/bulksupported.h

@@ -12,7 +13,8 @@ struct bulk_device_id {
 struct bulk_device_endpoints {
    std::uint8_t in_epaddr;
    std::uint8_t out_epaddr;
-    std::uint8_t interface_number;
+    // we may not know the interface, in which case we should not try to claim it.
+    std::optional<std::uint8_t> interface_number = std::nullopt;


The initializer is redundant. I would rather make it explicit in the table, that the value is unknown and we need to fill it to make it work on Windows and macOs. In addition a brief to-do comment in the table is useful. We may also file a bug as a reminder that these devices are not yet usable with windows.

are we sure they only don't work under windows?

I don't know. But it looks like @acolombier as added the claiming to enable the controllers in windows.

yeah I saw that. but maybe it also just didn't work under windows because libusb didn't claim the right interface automatically (since from what I could tell an interface needs to be claimed anyways in order to do any operation on it). Do we want to rely on the automatic claiming (if it exists across all backends at all)?

It looks like libusb is not able to automatically claim an interface. The auto feature is regarding detaching the kernel driver which is not supported on Windows.
But we need to test, I have no setup for that.

I guessed so because I saw auto_claim in winusb but upon further research its not used anywhere else. I don't have a setup for it either, but I guess we can continue not claiming it until someone with the setup files an issue.

Getting libusb running under Windows requires further changes. It is not functional yet, as we need to register a device driver (e.g. the generic WinUSB driver that comes with Windows) for the particular device type. This could be done using https://github.com/pbatard/libwdi but requires elevated user rights. Either in the Mixxx installer or at runtime of Mixxx itself with temporary elevated rights.
This is why BULK is default off for Windows.

acolombier · 2024-10-08T20:31:14Z

What's the status of this @Swiftb0y ? Do you need me to do further testing?

Swiftb0y · 2024-10-08T21:55:02Z

I'll look into it. In the meantime, there were a couple review comments we tagged you on @acolombier. I would appreciate if you could take a look at those.

Swiftb0y · 2024-10-09T09:38:59Z

@acolombier so I went through this again and I think its ready. Though I'd appreciate if you could test with your S4 to ensure I didn't break anything obvious.

acolombier · 2024-10-09T11:27:42Z

Thanks for looking into this. I will give that a test today.

daschuer

LGTM, Thank you.

acolombier · 2024-10-09T23:29:49Z

After a good 3 hours of fighting if Windows to just work, I'm afraid I won't be able to test it - the issue is with QML and GL support in the VM so not related to any of the changes, but required to prove the correct behaviour. I also couldn't borrow the Mac I was planning to test with.
I think overall the code looks fine so I'm happy with merging that change now, and test that in the future, maybe get someone from the community with a more realistic setup (aka not a VM) to test it!

daschuer · 2024-10-10T04:50:57Z

Ok, @Swiftb0y can you squash, than we can merge.

Swiftb0y · 2024-10-10T12:35:15Z

done. Thanks for the review

Swiftb0y · 2024-10-10T12:36:31Z

lets wait for CI though, just to be safe

* move equality comparision closer to type definition. * remove trailing null-entry in support array * modernize allowlist search

* replace magic (unnamed) values with proper constants * use idiomatic array iteration

JoergAtGithub · 2024-10-10T18:40:45Z

LGTM! Thank you!

Swiftb0y · 2024-10-10T19:50:52Z

Thank you.

github-actions bot added the controller backend label Sep 2, 2024

JoergAtGithub reviewed Sep 2, 2024

View reviewed changes

src/controllers/hid/hiddenylist.h Outdated Show resolved Hide resolved

JoergAtGithub reviewed Sep 2, 2024

View reviewed changes

src/controllers/bulk/bulksupported.h Outdated Show resolved Hide resolved

Swiftb0y requested a review from JoergAtGithub September 4, 2024 12:52

Swiftb0y mentioned this pull request Sep 4, 2024

modernize ControllerManager #13627

Open

daschuer reviewed Sep 5, 2024

View reviewed changes

src/controllers/bulk/bulkcontroller.cpp Outdated Show resolved Hide resolved

daschuer reviewed Sep 5, 2024

View reviewed changes

src/controllers/bulk/bulkcontroller.cpp Outdated Show resolved Hide resolved

daschuer reviewed Sep 5, 2024

View reviewed changes

src/controllers/bulk/bulkcontroller.cpp Outdated Show resolved Hide resolved

JoergAtGithub reviewed Sep 6, 2024

View reviewed changes

src/controllers/bulk/bulksupported.h Outdated Show resolved Hide resolved

JoergAtGithub reviewed Sep 6, 2024

View reviewed changes

src/controllers/bulk/bulksupported.h Outdated Show resolved Hide resolved

JoergAtGithub approved these changes Sep 6, 2024

View reviewed changes

daschuer reviewed Sep 8, 2024

View reviewed changes

src/controllers/bulk/bulkenumerator.cpp Outdated Show resolved Hide resolved

daschuer requested changes Sep 8, 2024

View reviewed changes

mixxxdj deleted a comment from daschuer Sep 9, 2024

daschuer requested changes Sep 10, 2024

View reviewed changes

daschuer reviewed Sep 13, 2024

View reviewed changes

Swiftb0y force-pushed the refactor/modernize-hid-bulk-lists branch from e6ed48b to 18c8ce4 Compare September 18, 2024 10:01

daschuer reviewed Sep 18, 2024

View reviewed changes

JoergAtGithub mentioned this pull request Oct 8, 2024

Prevent deadlock with BULK transfert and reduce log noise #13735

Merged

Swiftb0y requested a review from daschuer October 9, 2024 09:39

daschuer approved these changes Oct 9, 2024

View reviewed changes

Swiftb0y force-pushed the refactor/modernize-hid-bulk-lists branch from 60541eb to 45363b7 Compare October 10, 2024 12:35

Swiftb0y added 4 commits October 10, 2024 16:05

refactor: modernize bulksupport allowlist

af065df

* move equality comparision closer to type definition. * remove trailing null-entry in support array * modernize allowlist search

refactor: modernize hid_denylist

bbbbb1a

* replace magic (unnamed) values with proper constants * use idiomatic array iteration

refactor: restructure bulk_supported to emphasize kv structure

f034482

refactor: remove OS-specific hidapi code

45363b7

JoergAtGithub merged commit ae59b3f into mixxxdj:main Oct 10, 2024
13 checks passed

Swiftb0y deleted the refactor/modernize-hid-bulk-lists branch October 10, 2024 19:50

Modernize Hid/Bulk Lists #13622

Modernize Hid/Bulk Lists #13622

Conversation

Swiftb0y commented Sep 2, 2024

Swiftb0y commented Sep 6, 2024

JoergAtGithub commented Sep 6, 2024

Swiftb0y commented Sep 6, 2024

daschuer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Swiftb0y Sep 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Swiftb0y commented Sep 9, 2024

daschuer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JoergAtGithub Sep 16, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

daschuer commented Sep 11, 2024

Swiftb0y commented Sep 13, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Swiftb0y Sep 14, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Swiftb0y Sep 17, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

daschuer Sep 17, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Swiftb0y commented Sep 18, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

acolombier commented Oct 8, 2024

Swiftb0y commented Oct 8, 2024

Swiftb0y commented Oct 9, 2024

acolombier commented Oct 9, 2024

daschuer left a comment

Swiftb0y Sep 10, 2024 •

edited

Loading

JoergAtGithub Sep 16, 2024 •

edited

Loading

Swiftb0y Sep 14, 2024 •

edited

Loading

Swiftb0y Sep 17, 2024 •

edited

Loading

daschuer Sep 17, 2024 •

edited

Loading

Swiftb0y commented Sep 18, 2024 •

edited

Loading