-
Notifications
You must be signed in to change notification settings - Fork 1
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 example code showing how to create custom enums and structures #11
Conversation
5d70d0e
to
374e810
Compare
samples/demo-server/src/customs.rs
Outdated
VariableBuilder::new(&error_node, "stErrorData", "stErrorData") | ||
.organized_by(ObjectId::ObjectsFolder) | ||
.data_type(struct_id.clone()) | ||
//.value(Variant::ExtensionObject::ST_ErrorData::default()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@einarmo how do I do that? I am really unsure
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just .value(ExtensionObject::new(ST_ErrorData::default())
should work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that leads to the error:
|
23 | .value(ExtensionObject::new(error_data))
| -------------------- ^^^^^^^^^^ the trait opcua::opcua_types::MessageInfo
is not implemented for ErrorData
, which is required by ErrorData: DynEncodable
| |
| required by a bound introduced by this call
I can add implementation for these traits. But all other structs in opcua code seem to only derive from #[derive(Debug, Clone, PartialEq, opcua::types::BinaryEncodable, opcua::types::BinaryDecodable)]
as I do in that example. Am I missing something? any macro available?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok . they do not derive anything because they all implement that by hand... now I just tneed to remember what these values should be...
impl opcua::types::MessageInfo for WriteValue {
fn type_id(&self) -> opcua::types::ObjectId {
opcua::types::ObjectId::WriteValue_Encoding_DefaultBinary
}
fn json_type_id(&self) -> opcua::types::ObjectId {
opcua::types::ObjectId::WriteValue_Encoding_DefaultJson
}
fn xml_type_id(&self) -> opcua::types::ObjectId {
opcua::types::ObjectId::WriteValue_Encoding_DefaultXml
}
fn data_type_id(&self) -> opcua::types::DataTypeId {
opcua::types::DataTypeId::WriteValue
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well I am really not sure how to implement that... the custom structs Ids are not in the ObjectId enum for sure since I create them.
@einarmo ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Implement ExpandedMessageInfo
instead, then you can return ExpandedNodeId
s pointing to arbitrary namespaces, you don't even need to provide a namespace index, you can just use the full namespace.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The rust compiler is sometimes a little eager about inferring which traits you need to implement. ExpandedMessageInfo
is implemented for all types that implement MessageInfo
, but only core OPC-UA types can really implement MessageInfo
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok ExpandedNodeId is easier to return...
for the sample I can do whatever,
However in my case the namespace it defined at runtime by reading a config. I am really unsure how I can implement that without very ugly tricks..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess you could store it in a static? It's a bit of a niche requirement. Usually if the structure is static, meaning you define it at compile time, so is the namespace that defines the type.
Something like static MY_NAMESPACE: OnceCell<String>
could work, if storing it in a static is acceptable. If the namespace index is static you can return that instead. You could store it in the struct itself, but that will be hacky and potentially difficult to make work, so I wouldn't recommend it.
We need the namespace index in order to encode the extension object.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know what you're making or why, but I would recommend defining types and other shared components in a separate, static namespace the way all the companion standards do it.
samples/demo-server/src/customs.rs
Outdated
} | ||
} | ||
|
||
impl TryFrom<i32> for AxisState { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the entire i32 convertoin thing is really dum code. Can I use something like int-enum crate to do that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Making derive(BinaryEncodable, BinaryDecodable)
support repr
enums is on my TODO. That would be the cleanest way to do that IMO.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe a special trait for numeric enums with blanket implementations for ``IntoVariant,
TryFrom`, `BinaryEncodable`, etc...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that is exactly what the crate int-enum does. It has a trait IntEnum which does exactly that. Not tested for that particular case yet but I will.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, but we have slightly special requirements, and we probably want some other stuff as well to comply with how the standard demands we name enum variants.
I'll look into it, and look at int-enum
as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It also strikes me that blanket impls won't work, we still need to derive BinaryEncodable
, etc. in this case. There's a bigger effort to be done there to really extend the derive macros to cover more of the standard like unions, structs with optional types, and these enums...
374e810
to
4dcc3e2
Compare
FYI: #14, should let you clean up this a fair bit. |
nice! |
275b496
to
85d6b1e
Compare
@einarmo MR is ready to review and merge I think. tested with prosys client and python. Next step is to write some client code. I am starting with something like that. unless you tell me there is a better way
And then convertion will happen automatically? |
I think you also need to add the namespace, I want to create a way to read that from the server... You can also use |
Can we merge that MR? @einarmo Can I add a get_namespace_index method on Session struct then ? or you have a more precise idea of how to do it? |
Feel free, though we do need to add a way to actually read the namespace array from the server, haven't added that yet. That should be relatively simple. |
85d6b1e
to
263146b
Compare
263146b
to
1f27e9b
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. There is clearly room for improvement in the ergonomics of creating data types without codegen, but having this is a nice way to have an evolving picture of what it actually takes.
status:
code to generate UA enum and structure is correct (I think)
code to generate the rust objects is mostly correct
code to use the rust objects and set value of a variable is broken.
need to fix clipy things