Conversation
…program will not stop when interacting with the stick.
…es. This makes the software pluggable and gives us a better seperation of concern.
…ullable int values.
…properties on the BlinkStick class.
…interact using IUseDevice (not with a full BlinkStick). Renamed BlinkStickConnected and BlinkStickDisconnected because is should not reflect anything to do with BlinkStick.
… to do some syncing. Hopefully it won't be needed in the future.
…s now kept alive cross the application.
Now supporting parallel animations through threads
Moved some namespaces around
… sequential animation queueing to much loops.
…kes stuff more responsive.
|
Packages can be found here if you want to try them out: https://www.nuget.org/packages?q=blinkstick |
|
Any news on the matter? |
|
@KeesCBakker I'm hoping this project isn't dead. It'd be good to at least hear a response from the maintainer about your PR. Thanks for making the project available on NuGet. Are you using var monitor = new UsbMonitor();
monitor.Connected += (s, e) => Console.WriteLine("Connected");
monitor.Disconnected += (s, e) => Console.WriteLine("Disconnected");
monitor.Start(); |
| [assembly: AssemblyDescription("BlinkStick Generic Hid Library with KeesTalksTech modifications")] | ||
| [assembly: AssemblyConfiguration("")] | ||
| [assembly: AssemblyCompany("Agile Innovative Ltd")] | ||
| [assembly: AssemblyCompany("KeesTalksTech")] |
There was a problem hiding this comment.
Probably a bit presumptuous to change the company name on the assembly.
There was a problem hiding this comment.
Hahaha... I think it was an automatic rename. I'm moving the animations to another project.
There was a problem hiding this comment.
Has been a while. I did it to see the difference between the projects (the DLL from the Nuget package is not an official DLL).
| { | ||
| private double _hue; | ||
| private double _saturation; | ||
| private double _lightness; |
There was a problem hiding this comment.
If this is to be mutable, it should be a class not a struct.
https://blogs.msdn.microsoft.com/ericlippert/2008/05/14/mutating-readonly-structs/
My sense (which follows the BCL colour types) is to make HslColor and RbgColor immutable structs.
| /// Defines an HSL color. | ||
| /// </summary> | ||
| /// <remarks>Check http://en.wikipedia.org/wiki/HSL_color_space for more info.</remarks> | ||
| public struct HSLColor |
There was a problem hiding this comment.
.NET style guidelines would have this called HslColor.
There was a problem hiding this comment.
Ok -- I see that this type was moved, not added. Comment applies to upstream code, in that case.
There was a problem hiding this comment.
Yep, it has been restructured. A rename will break the implementation.
| // You should have received a copy of the GNU General Public License along with | ||
| // BlinkStick application. If not, see http://www.gnu.org/licenses/. | ||
| #endregion | ||
|
|
There was a problem hiding this comment.
You can't just remove the license header from files...
|
Took a few minutes to look through this PR. My $0.02 is that too much has changed for this to reasonably be merged. I get wide-reaching PRs from time to time against my own projects and generally they don't get merged. Even if there's some good stuff in there, it's too hard to see what's changed and whether it's correct. I think you'd have more luck with smaller PRs. A minimal PR that added NuGet support would be great. |
|
Perhaps you could also grant @arvydas access to the NuGet package. |
|
Thanks for the review. @arvydas can have all the rights to the project and the way packages are deployed. I'm also happy to work on a better way of merging the project. |
2 participants
Hi!
When I saw the BlinkStick code I saw it was very hard to maintain, that’s why I ended up restructuring its code. One thing led to another and I ended up with doing a lot of structural changes. I’ve tried to keep the methods as backwards compatible as possible.
Are you interested in merging the project?
I've removed my animations project. It can be a separate project (unless you're interested in async animations for the BlinkStick as well, happy to work together).
USB Monitoring
The project uses two dependencies, implemented by forked projects. I removed them for their NuGet packages. The code of these packages (the device and the stream) was highly coupled with BlinkStick. I ended up writing a IUsbDevice and IUsbStream abstractions so that packages can be easier swapped out in the future. I also split the monitor in a central monitor and a monitor for devices based on VendorId and ProductId. Note: I don’t have Linux/Mono so I cannot test it.
(Had to disable the signing otherwise it wouldn't work. You might want to look into that...)
Meta information
Created a wrapper for the meta information so it doesn’t clutter up the BlinkStick class. BlinkStick class still has the properties but reroutes it through the Meta property (decorated with obsoletes).
Animations
I’ve separated the animation logic by using an extension class. The user can still use the methods through the BlinkStick class, but the implementation now has its own class (Animations\AnimationExtensions.cs).
Colors
I did the same for the colors (Colors\ColorExtensions.cs). All SetColor methods are variations on the SetColor(channel, index, r, g, b).
Integration Testing
I had the need to for some integration tests (to test the USB monitor, test if the moved code still worked). That’s why I created a special project that helps debugging scenarios. Project is based on Visual Studio testing. Don’t know if Linux/Mono has something for it.
GetLedCount()
Now returns the right number of leds when a Square or other non-pro device is connected.
NuGet
For my own project I needed Nuget packages, so I implemented Continuous Integration with MyGet that helps in the creation of Nuget packages. Works like a charm… got the packages up. Love to transfer them to you.