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

Add support for controlling eebus-go via JSON-RPC from another process #128

Merged
merged 34 commits into from
Oct 28, 2024

Conversation

sthelen-enqs
Copy link
Contributor

This PR provides an implementation of a JSON-RPC 2
server that proxies calls into the eebus-go stack via TCP for control by other processes.

The current state of this PR is a draft, all basic (and some advanced)
functionality should work, but not all functions and edge cases are handled,
and those that are handled may not be implemented optimally. Consider this a
request for comments from the community on the current state of the PR and how
it should proceed.

The RPC server provides access to eebus-go methods via reflection and
static/dynamic method scoping. Static method scoping is used to proxy methods
for the eebus-go/service.Service struct by prefixing the method with
"service/", methods assigned to the RPC server itself are scoped under
"remote/", the service.LocalDevice methods are available under "localdevice/",
and all registered use cases are available via the registered usecaseId (e.g.
"eg-lpc/"). Dynamic method proxying is available currently for
DeviceRemoteInterface and EntityRemoteInterface types by prefixing the wanted
method with "Call/" and passing an AddressDeviceType or EntityAddressType as
the first parameter.

Use case specific notifications from the eebus-go stack are passed to the RPC
client via a json-rpc notification where the method field is set to the
api.EventType and the parameters is a JSON object containing the ski, the
device address and the device entity.

Notifications that the eebus-go stack normally provides the
ServiceHandlerInterface are proxied to the RPC client with the method set the
name of the called method in the ServiceHandlerInterface prefixed with the
string "remote/". The parameters for these notifications are notification
specific.

There are still several open points that I have either not yet implemented or
that I think should be discussed by the community before being addressed:

  • not all types returned by functions in the eebus-go stack can be easily
    serialized/deserialized via JSON. I have added automatic
    serialization/deserialization for the DeviceRemoteInterface and
    EntityRemoteInterface, but all other types that are Interfaces, contain
    Interfaces, or contain private members cannot currently be serialized or
    deserialized, this notably includes the DeviceLocalInterface, and the
    RemoteEntityScenarios struct. Options to increase the number of
    serializable/deserializable types include implementing the
    Marshaler/Unmarshaler interface for simple types (such as
    RemoteEntityScenarios) or adjusting method return types to make them
    serializable/deserializable.

  • it's impossible to pass a callback via JSON-RPC so all functions that can
    take a resultCB must currently take nil (json null) in that position. It may
    be nice to detect functions with such a parameter and provide a means for
    callers to register interest in the result of such an operation such that the
    RPC server creates a resultCB type that forwards the ResultDataType as a
    notification to the controlling client. To accomplish this, it would be
    necessary to adjust the type of the resultCB function to also take the
    MsgCounter as a second argument as the RPC server cannot create a function
    which knows the MsgCounter prior to the actual function in question being
    called.

I've already received some feedback to a few points that I will collect here:

  • currently all functions that return (value, error) will drop the error from
    the response when it is nil, and return a JSON-RPC error message otherwise.
    This handling does not occur for all functions with more than 2 return
    values. It may be more consistent to adjust this, or drop custom handling of
    errors entirely. Errors would then be returned as the final argument in their
    string representation.

  • currently the EEBus-Service is automatically started as soon as the JSON-RPC
    listener starts, it might be better if the service is manually started via a
    JSON-RPC method call instead

  • the RPC server currently does not limit the number of controlling
    connections, nor does it bind the lifetime of the EEBus service to the
    lifetime of the TCP connection. We may want to limit the number of
    simultaneous connections to 1, as well as terminate the EEBus service on
    connection loss to prevent e.g. limits from staying valid even though the
    controlling connection died

sthelen-enqs and others added 30 commits October 23, 2024 10:47
Allows unmarshalling structs instead of unmarshalling to map
Allows registering e.g. EG-LPC and CS-LPC on the same remote
This is useful so we can call UnregisterRemoteSKI for unknown/unwanted devices
According to spec, the field may be omitted and should be treated as
calling a function without parameters.
cmd/remote/rpc.go Fixed Show resolved Hide resolved
cmd/remote/rpc.go Fixed Show fixed Hide fixed
cmd/remote/rpc.go Fixed Show fixed Hide fixed
cmd/remote/rpc.go Fixed Show fixed Hide fixed
cmd/remote/ucs.go Fixed Show resolved Hide resolved
cmd/remote/ucs.go Fixed Show fixed Hide fixed
cmd/remote/util.go Dismissed Show dismissed Hide dismissed
cmd/remote/util.go Dismissed Show dismissed Hide dismissed
Sending a notification can only fail if JSON serialization fails, or if
writing to the socket fails.

We can't handle JSON serialization failures here, and if writing to the
socket fails the connection should be dropped by the json-rpc
implementation anyway.
@DerAndereAndi DerAndereAndi added the enhancement New feature or request label Oct 27, 2024
All eebus-go functions will now return a JSON array which includes all
return parameters including any error values.

NOTE: This is a breaking change
@DerAndereAndi
Copy link
Member

I would suggest that we merge this PR as is (with the change request mentioned above), as it is implemented as an example command for now.

The next step would be to great a new long running feature branch e.g. feature/jsonrpc2 that I would start and an according issue. This would then be the basis for moving json-rpc2 support as an API, including test-coverage. That branch will then be the basis for PRs working on that.

RegisterUseCase.

These errors are currently always API misuse, but it might be nice to
extend RegisterUseCase to allow returning errors anyway.
@sthelen-enqs
Copy link
Contributor Author

Moving the development into a branch here should make it easier for others to contribute as well, sounds like a good idea to me.

@sthelen-enqs sthelen-enqs marked this pull request as ready for review October 28, 2024 16:13
@DerAndereAndi DerAndereAndi merged commit a37178d into enbility:dev Oct 28, 2024
4 checks passed
@sthelen-enqs sthelen-enqs deleted the feature/jsonrpc2-api branch October 29, 2024 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants