-
Notifications
You must be signed in to change notification settings - Fork 122
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
Publicly export ExceptionResponse
and Exception
types
#218
Conversation
Please rebase on main to fix the unrelated pre-commit issues. |
How about a code example? Could we add error handling to one of the existing examples or (even better) a doctest example? |
Yep that's a good idea. Will add it tomorrow! |
@@ -38,7 +38,9 @@ pub use self::slave::{Slave, SlaveId}; | |||
mod codec; | |||
|
|||
mod frame; | |||
pub use self::frame::{Address, FunctionCode, Quantity, Request, Response}; | |||
pub use self::frame::{ | |||
Address, Exception, ExceptionResponse, FunctionCode, Quantity, Request, Response, |
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 think we should make ExceptionResponse
public but not its fields.
We could change the fields to pub(crate)
in order to still be able to access it in the crate.
We should add at least add 2 methods for the ExceptionResponse
:
exception()
that returns theException
function_code()
that returns theFunctionCode
And we should also enable a simple creation of the ExceptionResponse
with a new
method, or a method on Request
/ Response
maybe to create a ExceptionResponse
from it.
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.
After thinking about it more, I think only Exception
should be in the public API.
The user shouldn't have to know the existence of ExceptionResponse
, the server should be able to build the appropriate Response
type (either Response
or ExceptionResponse
) by the return of the call
function.
See #237 for more context.
Closing in favor of #246. |
By making these types public, one is able to get the specific modbus error from a Modbus server.
So this is a short-term solution for #169, but it's definitely not a good one.