Skip to content
This repository has been archived by the owner on Oct 12, 2023. It is now read-only.

[.NET binding] Suggestions about .NET binding #20

Closed
yfakariya opened this issue Aug 12, 2016 · 3 comments
Closed

[.NET binding] Suggestions about .NET binding #20

yfakariya opened this issue Aug 12, 2016 · 3 comments

Comments

@yfakariya
Copy link
Contributor

Hi, thank you for great work!
I know this is still beta, but I found some improvements about .NET binding.

  1. [Suggestion] Add .NET Core support. Although coreclr itself does not support ARM linux, it is valuable to support .NET Core as well as UWP. I found that some hosting API does not exist in coreclr of non Windows builds, but I think it can be solved moving instantiation of managed objects from C++ to C# and uses static methods to communicate via coreclr_create_delegate API.
  2. [Suggestion] Use class based API like the Java binding. The IGatewayModule is interface now, it has following problems:
    • API usability. App developers must always implement all methods even if they want to only one method. SensorModule sample code is good example for it. Base class with nop virtual methods can solve this.
    • API's backward compatibility. The interface is not backward compatible.
    • API design restriction. It prevents distinguish between internal APIs(SPIs) to communicate for native parts and public APIs to communicate to Apps. For example, internal APIs can expose byte* based APIs to avoid redundant copies of arrays.
  3. [Improvement] Use IntPtr instead of long for pointer or handle types in managed code in general.
  4. [Bug] Do not receive UTF-8 encoded string as String in managed code (namely, configuration parameter of IGatewayModule.Create). The string in variant_t will be considered as ANSI encoded string, so unexpected behavior will be caused. I think it is easier to passing string as a byte array and decode it in managed code. This is repro code:
// in SensorModule sample class, for example
public void Create(MessageBus bus, string configuration)
{
    // This line should out "This is C# Sensor Module Create 'モジュール構成' even if code page is not 65001.
    Console.WriteLine($"This is C# Sensor Module Create '{configuration}'!");
}
{
    "modules" :
    [
        {
            "module name" : "logger_hl",
            "module path" : "..\\..\\..\\modules\\logger\\Debug\\logger_hl.dll",
            "args" : {"filename":"C:\\Temp\\Log.txt"}
        },
        {
          "module name": "dotnet_sensor_module",
          "module path": "..\\..\\..\\bindings\\dotnet\\Debug\\dotnet_hl.dll",
          "args": {
            "dotnet_module_path": "SensorModule",
            "dotnet_module_entry_class": "SensorModule.DotNetSensorModule",
            "dotnet_module_args": "モジュール構成"
          }
        },
        {
            "module name" : "dotnet_printer_module",
            "module path" : "..\\..\\..\\bindings\\dotnet\\Debug\\dotnet_hl.dll",
            "args" : {
                "dotnet_module_path": "PrinterModule",
                "dotnet_module_entry_class": "PrinterModule.DotNetPrinterModule",
                "dotnet_module_args": "module configuration"
            }
        }
    ]
}
@aribeironovaes
Copy link
Contributor

Thanks a lot Yusuke!

These are great feedback.

Can I ask you to open separate Issues on github for each of this so we can
track the resolution separately?

Again, thanks a lot,

Angelo Ribeiro.

On Fri, Aug 12, 2016 at 1:44 AM, Yusuke Fujiwara notifications@github.com
wrote:

Hi, thank you for great work!
I know this is still beta, but I found some improvements about .NET
binding.

  1. [Suggestion] Add .NET Core support. Although coreclr itself does
    not support ARM linux, it is valuable to support .NET Core as well as UWP.
    I found that some hosting API does not exist in coreclr of non Windows
    builds, but I think it can be solved moving instantiation of managed
    objects from C++ to C# and uses static methods to communicate via
    coreclr_create_delegate API.
  2. [Suggestion] Use class based API like the Java binding. The
    IGatewayModule is interface now, it has following problems:
    • API usability. App developers must always implement all methods
      even if they want to only one method. SensorModule sample code is
      good example for it. Base class with nop virtual methods can solve this.
    • API's backward compatibility. The interface is not backward
      compatible
      https://msdn.microsoft.com/en-us/library/ms229013(v=vs.100).aspx.
    • API design restriction. It prevents distinguish between internal
      APIs(SPIs) to communicate for native parts and public APIs to communicate
      to Apps. For example, internal APIs can expose byte* based APIs to
      avoid redundant copies of arrays.
  3. [Improvement] Use IntPtr instead of long for pointer or handle
    types in managed code in general.
  4. [Bug] Do not receive UTF-8 encoded string as String in managed code
    (namely, configuration parameter of IGatewayModule.Create). The string
    in variant_t will be considered as ANSI encoded string, so unexpected
    behavior will be caused. I think it is easier to passing string as a byte
    array and decode it in managed code. This is repro code:

// in SensorModule sample class, for examplepublic void Create(MessageBus bus, string configuration)
{
// This line should out "This is C# Sensor Module Create 'モジュール構成' even if code page is not 65001.
Console.WriteLine($"This is C# Sensor Module Create '{configuration}'!");
}

{
"modules" :
[
{
"module name" : "logger_hl",
"module path" : "......\modules\logger\Debug\logger_hl.dll",
"args" : {"filename":"C:\Temp\Log.txt"}
},
{
"module name": "dotnet_sensor_module",
"module path": "......\bindings\dotnet\Debug\dotnet_hl.dll",
"args": {
"dotnet_module_path": "SensorModule",
"dotnet_module_entry_class": "SensorModule.DotNetSensorModule",
"dotnet_module_args": "モジュール構成"
}
},
{
"module name" : "dotnet_printer_module",
"module path" : "......\bindings\dotnet\Debug\dotnet_hl.dll",
"args" : {
"dotnet_module_path": "PrinterModule",
"dotnet_module_entry_class": "PrinterModule.DotNetPrinterModule",
"dotnet_module_args": "module configuration"
}
}
]
}


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
#20, or mute the
thread
https://github.com/notifications/unsubscribe-auth/AI75b7MQqZAWt9A6hH6MgRgG_F74vNvnks5qfDJ-gaJpZM4Ji4uD
.


Angelo Ribeiro
Msc - Federal University of Pernambuco
Team in Training Alumni Triple Crowned! :)

@yfakariya
Copy link
Contributor Author

@aribeironovaes Sure! I separated above four issues into individual issues. So, I'll close this issue.

@aribeironovaes
Copy link
Contributor

Thanks a lot again Yusuke!
We will triage and label each item accordingly.
Angelo Ribeiro

Get Outlook

    _____________________________

From: Yusuke Fujiwara notifications@github.com
Sent: Friday, August 12, 2016 4:35 PM
Subject: Re: [Azure/azure-iot-gateway-sdk] [.NET binding] Suggestions about .NET binding (#20)
To: Azure/azure-iot-gateway-sdk azure-iot-gateway-sdk@noreply.github.com
Cc: Mention mention@noreply.github.com, Angelo Ribeiro aribeironovaes@gmail.com

@aribeironovaes Sure! I separated above four issues into individual issues. So, I'll close this issue.


You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants