Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move to System.IO.Ports, combine projects into a single NuGet package #29

Merged
merged 14 commits into from
Jun 6, 2022

Conversation

julian-baumann
Copy link
Contributor

Solves a part of #22.
Changed the SerialConnector to use the cross-platform System.IO.Ports package, because the earlier used EnhancedSerialConnector raised an exception when trying to use it with .NET 5/6 on Linux or macOS.

This PR also aims to combine the OBD.NET.Desktop with the OBD.NET.Common project, to simplify the use of this package. The code was also refactored to C# 10.

The OBD.NET.Universal project should continue to work with this implementation, though I couldn't test it on my own due to a lack of the required hardware. It build successfully and I couldn't see any reasons, why it shouldn't continue to work.

The new OBD.NET package was tested on macOS/Linux (Raspberry Pi) and worked just fine.

Note: I also renamed the NuGet package in the csproj file from OBD.NET.Common to OBD.NET

@DarthAffe
Copy link
Owner

Hey, thanks for contributing :)

To give you a status update on this:
I'm done looking through the changes, but with the removal of the enhanced serial port you broke everything < .net 5 - I'm currently fixing this.
Sadly my car-pc died recently which makes testing currently a bit of a hassle - i'll need a bit more time here :)

@julian-baumann
Copy link
Contributor Author

Ahh didn't check for that.
Also sorry for the incredible messy PR. I should've created a separate PR regarding the System.IO.Ports changes. Must've been hard to review the changes since the renaming of the projects probably broke the git diff in a way.

@DarthAffe
Copy link
Owner

Finally got some time to finish reviewing and testing this - thanks again for contributing :)

This should indeed fix #22

@DarthAffe DarthAffe merged commit d66b812 into DarthAffe:master Jun 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants