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

return error codes to application #669

Merged
merged 17 commits into from
May 18, 2023

Conversation

chrimaka
Copy link
Contributor

@chrimaka chrimaka commented Apr 9, 2023

That's a PR for passing the error codes from the stack to the application. It requires a bit more fine tuning (null checks; casting of CBErrors from iOS). But this would be the general concept. Feedback very welcome.

@chrimaka chrimaka changed the title 668 return error codes return error codes to application Apr 9, 2023
@janusw
Copy link
Member

janusw commented Apr 10, 2023

Thanks for the PR! Unfortunately the build fails with errors like this:

build: Source/Plugin.BLE/Windows/Characteristic.cs#L50
'Characteristic.ReadNativeAsync()': return type must be 'Task<Tuple<byte[], int>>' to match overridden member 'CharacteristicBase<GattCharacteristic>.ReadNativeAsync()'

Please fix! :)

@chrimaka
Copy link
Contributor Author

chrimaka commented Apr 10, 2023

@janusw I have a question on the build process. I was able to build the former 2.1.x "out of the box". I also can build the Plugin.BLE.sln without issues, but If I try to build BLE.sln, VS2022 for macOS (17.5.3 (build 15)) already gives me an error during opening error NETSDK1045: Das aktuelle .NET SDK unterstützt .NET Core 6.0 nicht als Ziel. Geben Sie entweder .NET Core 5.0 oder niedriger als Ziel an, oder verwenden Sie eine .NET SDK-Version, die .NET Core 6.0 unterstützt. Für das Projekt kann keine Paketspezifikation erstellt werden.

I should have the relevant SDKs installed:

dotnet --list-sdks
6.0.404 [/usr/local/share/dotnet/sdk]
6.0.405 [/usr/local/share/dotnet/sdk]
6.0.406 [/usr/local/share/dotnet/sdk]
6.0.407 [/usr/local/share/dotnet/sdk]
7.0.101 [/usr/local/share/dotnet/sdk]
7.0.102 [/usr/local/share/dotnet/sdk]
7.0.103 [/usr/local/share/dotnet/sdk]
7.0.200 [/usr/local/share/dotnet/sdk]
7.0.202 [/usr/local/share/dotnet/sdk]

dotnet currently points to 7.0.202 and msbuild is installed with version 16.10.1.60101. As specified in this help article I tried turn on/off using msbuild on mono, but without any success. (I'm building on macOS). Any idea how to overcome this issue?

@janusw
Copy link
Member

janusw commented Apr 10, 2023

@janusw I have a question on the build process. I was able to build the former 2.1.x "out of the box". I also can build the Plugin.BLE.sln without issues, but If I try to build BLE.sln, VS2022 for macOS (17.5.3 (build 15)) already gives me an error during opening error NETSDK1045: Das aktuelle .NET SDK unterstützt .NET Core 6.0 nicht als Ziel. Geben Sie entweder .NET Core 5.0 oder niedriger als Ziel an, oder verwenden Sie eine .NET SDK-Version, die .NET Core 6.0 unterstützt. Für das Projekt kann keine Paketspezifikation erstellt werden.

I'm afraid that's some sort of deficiency of VS4Mac. VS on Windows should work AFAIK. Maybe we can find a workaround ...

Copy link
Member

@janusw janusw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall this looks pretty good to me, but I have some minor comments ...

Source/Plugin.BLE/Shared/Contracts/ICharacteristic.cs Outdated Show resolved Hide resolved
Source/Plugin.BLE/Shared/Contracts/ICharacteristic.cs Outdated Show resolved Hide resolved
Source/Plugin.BLE/Android/GattCallback.cs Outdated Show resolved Hide resolved
@janusw janusw mentioned this pull request Apr 30, 2023
@janusw
Copy link
Member

janusw commented May 5, 2023

@janusw I have a question on the build process. I was able to build the former 2.1.x "out of the box". I also can build the Plugin.BLE.sln without issues, but If I try to build BLE.sln, VS2022 for macOS (17.5.3 (build 15)) already gives me an error during opening

This is fixed (or rather worked around) by now via #685. Using BLE.Mac.slnf should work on Mac (however, it only includes the core libraries, but not all the sample clients).

@janusw
Copy link
Member

janusw commented May 7, 2023

@chrimaka Could you take care of the remaining comments, or should I fix them (so that we can move forward with this)? It would be nice if we could merge it soon.

@chrimaka
Copy link
Contributor Author

@janusw sorry for the late reply. I was out sick for some time. I'll look into this on Thrusday/Friday + Weekend.

Copy link
Member

@janusw janusw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, thanks for addressing all my comments. This is ok for master by now.

@janusw janusw merged commit 77ee52e into dotnet-bluetooth-le:master May 18, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants