Suggestion for message types. #56

BenediktBurger · 2023-07-12T10:46:02Z

I added message types and error codes.

They are based on the current pyleco implementation pymeasure/pyleco#3

If you prefer looking at messages in python style, have a look here: https://github.com/pymeasure/pyleco/blob/add-core/pyleco/core/leco_protocols.py

As we follow openrpc, I figured, we should define the methods in openrpc style and not as python methods.

Closes #29

ToDo:

Relate messages of the transfer part like "CO_SIGNIN" to the method "coordinator_sign_in"
Use "summary" instead of "description" for a first description of methods.

bilderbuchi

I went through this, I had a lot of question, mainly on the openrpc stuff -- it seems curiously restricted in expressiveness, but probably that's because of lack of familiarity.

control_protocol.md

bilderbuchi · 2023-07-22T10:30:57Z

control_protocol.md

+:::
+
+Any Component MAY offer ANY of the following methods.
+Components SHOULD offer ``shut_down``.


I'm curious about the optional methods. What happens if some component calls another one's shutdown, but that is not implemented? Do we need some common fallback method? Do you get a not implemented error back or somesuch?

Do you get a not implemented error back or somesuch?

jsonrpc has a "method not found" error defined.

I'm curious about the optional methods.

My reasoning is: There are some methods necessary, such that a Component fulfills it role (as a component, it has to be pingable), these are necessary.

Some features are nice to have, for example logging is not essential (maybe the Component does not have logging capabilities).

Yes, I realise why optional methods can be useful. We should have a clear approach how do deal with that optionality, though -- there are several possibilities (do we error? ignore? publish/discover/supply a list of implemented methods? how should the initiating party deal with that? ...)

Components MUST adhere to openRPC, therefore they MUST offer a "rpc.discover" methods, which tells about all the implemented methods (the output is exactly that, what is part of this PR).

Should we include this method in the Components list (together with "pong")?

If you try to call a method, a Component does not have, you get an appropriate error.

So, you (as a user) can either query beforehand and use only methods present, or you can try a method name and handle the error.

Should we include this method in the Components list (together with "pong")?

IMO, yes.

So, you (as a user) can either query beforehand and use only methods present, or you can try a method name and handle the error.

It's more a LECO implementor that has to deal with that. As we are designing for different implementations to talk to each other, I'm wondering if we should standardise the expectations around what happens with optional methods. The answer might as well be "no", or "use what rpc.discover gives you as you see fit".

"no", or "use what rpc.discover gives you as you see fit".

That's my stance 😁

control_protocol.md

BenediktBurger · 2023-07-22T22:22:59Z

One informational note: I generated the json texts by calling the "rpc.discover" method (defined by openrpc) of the different Components, which returns (per definition) the methods of the (python) Components in openrpc json format.

If I made mistakes in the pyleco components (type hinting) or if the jsonrpc developer made errors, they reflect here.

bilderbuchi · 2023-07-23T09:22:54Z

One informational note: I generated the json texts by calling the "rpc.discover" method (defined by openrpc) of the different Components, which returns (per definition) the methods of the (python) Components in openrpc json format.

If I made mistakes in the pyleco components (type hinting) or if the jsonrpc developer made errors, they reflect here.

Ah, that makes this clearer! Makes sense.

bilderbuchi · 2023-07-23T09:23:13Z

In general, this RPC thing sometimes feels a little "inverted" to me somehow....

Coordinator A calls a method list_connected_components of Coordinator B,
not to make Coordinator B list its components via its own method, which you would expect if you just had an API reference of the Coordinator -- B.list_connected_components() would not take arguments, but return a list of connected components as per its name.

If A calls a method of B to tell B about A's details, that method would be something like B.add_connection_set(A, A's components). But for a message type/command name, this feels weird again.

So, inherently, I sometimes feel this mismatch between the names of commands, and those same names being method names in another object. I think it's an indication that the names or interfaces could be improved 😕

Mind you, this is not always the case -- a Coordinator calling an Actor's set_parameters(paramname, value)/sending a message SET_PARAM paramname value feels just right. 🤷

BenediktBurger · 2023-07-24T10:22:49Z

In general, this RPC thing sometimes feels a little "inverted" to me somehow....

I agree. The most prominent (and simple) example is "ping": I send a ping message and expect a pong response.
With RPC I call the responding method, which should be called "pong", therefore.

If A calls a method of B to tell B about A's details, that method would be something like B.add_connection_set(A, A's components). But for a message type/command name, this feels weird again.

I see your point.
To complicate things, we dont supply AinB.add_connection_set(A, A's components), because B` can deduce it from the message header. That might add to the confusion.

The Coordinator messages (regarding the directory) are the least obvious ones and require some collaboration.

Maybe we can use names like (these are methods in Coordinator A):
set_nodes (method tries to connect to other nodes, if not already connected)
get_nodes (returns nodes it is connected to)
set_component_directory (sets the component part of the directory, for example B's Components)
get_component_directory (gets all the components known to A, even those connected to B)
get_local_directory (gets Components connected to A

BenediktBurger · 2023-07-24T11:02:44Z

To store openRPC links globally, not in discussion:

specification
A playground to test an openRPC spec file / text snippet: It shows on one side the code, on the other side you can browse the resulting specificiation visually.

BenediktBurger · 2023-07-24T11:11:33Z

Note to myself:

Define errors in methods, if appropriate, see https://spec.open-rpc.org/#method-object regarding errors. Especially the Coordinator Sign in can define the "DUPLICATE_NAME" error.

Co-authored-by: Christoph Buchner <[email protected]>

bilderbuchi · 2023-07-24T15:35:37Z

In general, this RPC thing sometimes feels a little "inverted" to me somehow....

I agree. The most prominent (and simple) example is "ping": I send a ping message and expect a pong response. With RPC I call the responding method, which should be called "pong", therefore.

If A calls a method of B to tell B about A's details, that method would be something like B.add_connection_set(A, A's components). But for a message type/command name, this feels weird again.

I see your point. To complicate things, we dont supply AinB.add_connection_set(A, A's components), because B` can deduce it from the message header. That might add to the confusion.

The Coordinator messages (regarding the directory) are the least obvious ones and require some collaboration.

Yeah untangling this could be a bit tiresome, but clear names are worth it. I hope we manage to unify the "command"/"message" character with the "method" being called in most cases.

Maybe we can use names like (these are methods in Coordinator A):
set_nodes (method tries to connect to other nodes, if not already connected)
get_nodes (returns nodes it is connected to)
set_component_directory (sets the component part of the directory, for example B's Components)
get_component_directory (gets all the components known to A, even those connected to B)
get_local_directory (gets Components connected to A

Maybe moving away from a simple get/set prefix will yield some benefits. If I understand your list correctly, the following could work better (here I'm assuming A sends to B, I'm not sure if your examples were the other way round):

get_nodes -> send_nodes. A sends to B: send_nodes. B sends its list of nodes.
set_nodes -> add_nodes. A sends to B: add_nodes(A's-list-of-nodes). B unifies its list of nodes with the ones just received
set_component_directory: Semantically, it feels weird if A could force B to set its component directory to something, as A is an external entity. Maybe add_components or record_components: A sends to B: record_components(list-of-A's-components). B records those components into its directory, making use of the fact that it knows they came from A.
get_component_directory -> send_component_directory, same logic as for send_nodes.
get_local_directory -> send_local_directory, same logic as for send_nodes.

BenediktBurger · 2023-07-24T15:46:26Z

environment.yml

@@ -7,3 +7,4 @@ dependencies:
  - sphinx=5.3.0
  - sphinxcontrib-mermaid=0.8.1
  - sphinx_rtd_theme=1.1.1
+  - sphinx-jsonschema=1.19.1


I don't get jsonschema to work. It still shows .. jsonschema:: filename.json instead of the content...

I've never used it before. Maybe an unexpected interaction with myst parser?
Is the file path correct? Is the extension really loaded (such that the directive gets acted upon)? no idea otherwise.

Maybe an unexpected interaction with myst parser?

I commented the myst parser (and mermaid), but to no avail.

Is the file path correct?

Yes

Is the extension really loaded (such that the directive gets acted upon)?

At least it does not show any errors locally or in the CI.

I even used the jsonschema in the rst file from: https://sphinx-jsonschema.readthedocs.io/en/latest/directive.html#usage
It does not work. I don't know 🤷

I got it to work:
In an markdown file you have to use a colon fence:

:::{jsonschema} your schema :::

This does not work, however, for referencing files, only for showing a code snippet.

It does not render our schemata nicely 😭 (it does not render anything at all, as it does not recognize the keywords...).

well, some progress at least I guess? :-/

I went with the data-viewer, that works better (but does not allow examples...).

schemas/actor.json

BenediktBurger · 2023-07-24T15:49:18Z

schemas/component_optional.json

+                    "schema": {
+                        "type": "integer",
+                        "multipleOf": 10,
+                        "minimum": 0,
+                        "maximum": 100
+                    },


See #56 (comment)

No, I mean the parameter of the method
"name": "level",
"schema": {
"type": "integer" "one-of-info/debug/warning/error/critical"

Idea is to change to a set of possible values. I don't know yet, how to define it properly.

I found a way to define a set of possible values in json-schema ( "type": "integer", "enum": [1, 5, 9]).
Now the question is, do we want to have error levels as an integer or as a string?
If an integer, how do we define it (copy and or link to the python definitions at https://docs.python.org/3/library/logging.html#logging-levels)?

Hm, for language (and logging-framework) independence, I think a fixed set of level names seems most clear (the python ones seem fine to me).

I changed it to strings.

BenediktBurger · 2023-07-24T15:50:58Z

schemas/coordinator.json

+            "errors": [
+                {
+                    "code": -32091,
+                    "message": "The name is already taken.",
+                    "data": "Already taken name."
+                }
+            ]


Connect it to the error definition, lest this definition and the error list diverge.

schemas/coordinator.json

schemas/actor.json

BenediktBurger · 2023-07-24T17:38:14Z

I added examples to the dictionary methods to better understand how they work.

schemas/coordinator.json

BenediktBurger · 2023-09-14T13:15:38Z

Different idea to show json: https://github.com/useblocks/sphinx-data-viewer
Or, as a backup, simply json annotation in sphinx: json-object

schemas/actor.json

…sage names. Closes #57

BenediktBurger · 2024-08-20T15:06:30Z

I noticed, that there is a "summary" field for a method, not only description. We should use that instead (in many cases).

Suggestion for message types.

ee81fc5

BenediktBurger added this to the Message payload content serialization version 0 milestone Jul 12, 2023

Adjust error code ranges and numbers.

baae9af

bilderbuchi requested changes Jul 22, 2023

View reviewed changes

BenediktBurger and others added 2 commits July 24, 2023 13:27

Error messages improved.

5ae6e5b

Co-authored-by: Christoph Buchner <[email protected]>

Moving schemas to their own files and use sphinx-jsonschema.

2881646

BenediktBurger commented Jul 24, 2023

View reviewed changes

schemas/actor.json Outdated Show resolved Hide resolved

BenediktBurger commented Jul 24, 2023

View reviewed changes

schemas/coordinator.json Outdated Show resolved Hide resolved

BenediktBurger commented Jul 24, 2023

View reviewed changes

schemas/actor.json Outdated Show resolved Hide resolved

BenediktBurger added 2 commits July 24, 2023 18:04

Rename methods.

4254a2b

Switching to dictionaries, examples added.

9ed49b2

BenediktBurger commented Jul 31, 2023

View reviewed changes

schemas/coordinator.json Outdated Show resolved Hide resolved

Reword add_notes description

d056970

BenediktBurger commented Sep 20, 2023

View reviewed changes

schemas/actor.json Outdated Show resolved Hide resolved

BenediktBurger added 4 commits September 26, 2023 15:42

Changing to data-view.

9d9a6d7

Add locking actor.

50f3849

Change message names in the control protocol according to the RPC mes…

5ce9ab6

…sage names. Closes #57

Make data-view happy (removing examples).

3bcbfcd

BenediktBurger linked an issue Oct 11, 2023 that may be closed by this pull request

Connect message names with RPC calls #57

Open

BenediktBurger mentioned this pull request Oct 11, 2023

Connect message names with RPC calls #57

Open

BenediktBurger added 2 commits October 11, 2023 10:37

Change logging levels to strings.

a744a4e

Fix environment.yml.

b4de36c

BenediktBurger requested a review from bilderbuchi October 11, 2023 08:49

BenediktBurger mentioned this pull request Oct 31, 2023

Actor message format for Channels #63

Open

Improve actor.json according to current Actor/Device definitions

12d4589

BenediktBurger force-pushed the message-types branch from 6e44556 to 12d4589 Compare November 1, 2023 12:05

Fix typo.

95c2c69

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Suggestion for message types. #56

Suggestion for message types. #56

BenediktBurger commented Jul 12, 2023 •

edited

Loading

bilderbuchi left a comment •

edited

Loading

bilderbuchi Jul 22, 2023

BenediktBurger Jul 22, 2023

bilderbuchi Jul 23, 2023 •

edited

Loading

BenediktBurger Jul 24, 2023 •

edited

Loading

bilderbuchi Jul 24, 2023

BenediktBurger Jul 24, 2023

BenediktBurger commented Jul 22, 2023

bilderbuchi commented Jul 23, 2023

bilderbuchi commented Jul 23, 2023 •

edited

Loading

BenediktBurger commented Jul 24, 2023

BenediktBurger commented Jul 24, 2023 •

edited

Loading

BenediktBurger commented Jul 24, 2023 •

edited

Loading

bilderbuchi commented Jul 24, 2023

BenediktBurger Jul 24, 2023

bilderbuchi Jul 24, 2023

BenediktBurger Jul 24, 2023

BenediktBurger Jul 24, 2023

BenediktBurger Sep 12, 2023

bilderbuchi Sep 23, 2023 •

edited

Loading

BenediktBurger Oct 11, 2023

BenediktBurger Jul 24, 2023 •

edited

Loading

BenediktBurger Sep 6, 2023

bilderbuchi Sep 6, 2023

BenediktBurger Oct 11, 2023

BenediktBurger Jul 24, 2023

BenediktBurger commented Jul 24, 2023

BenediktBurger commented Sep 14, 2023

BenediktBurger commented Aug 20, 2024

Suggestion for message types. #56

Are you sure you want to change the base?

Suggestion for message types. #56

Conversation

BenediktBurger commented Jul 12, 2023 • edited Loading

bilderbuchi left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bilderbuchi Jul 23, 2023 • edited Loading

Choose a reason for hiding this comment

BenediktBurger Jul 24, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BenediktBurger commented Jul 22, 2023

bilderbuchi commented Jul 23, 2023

bilderbuchi commented Jul 23, 2023 • edited Loading

BenediktBurger commented Jul 24, 2023

BenediktBurger commented Jul 24, 2023 • edited Loading

BenediktBurger commented Jul 24, 2023 • edited Loading

bilderbuchi commented Jul 24, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bilderbuchi Sep 23, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BenediktBurger Jul 24, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BenediktBurger commented Jul 24, 2023

BenediktBurger commented Sep 14, 2023

BenediktBurger commented Aug 20, 2024

BenediktBurger commented Jul 12, 2023 •

edited

Loading

bilderbuchi left a comment •

edited

Loading

bilderbuchi Jul 23, 2023 •

edited

Loading

BenediktBurger Jul 24, 2023 •

edited

Loading

bilderbuchi commented Jul 23, 2023 •

edited

Loading

BenediktBurger commented Jul 24, 2023 •

edited

Loading

BenediktBurger commented Jul 24, 2023 •

edited

Loading

bilderbuchi Sep 23, 2023 •

edited

Loading

BenediktBurger Jul 24, 2023 •

edited

Loading