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

Inconsistent API for creating, opening, closing, and destroying channels #6

Closed
volks73 opened this issue Mar 26, 2020 · 3 comments
Closed
Assignees
Labels
bug Something isn't working enhancement New feature or request

Comments

@volks73
Copy link
Member

volks73 commented Mar 26, 2020

See the comment from #3, but basically there is a Close VI for a channel but no complementary Open VI. The toolkit generally uses the following naming conventions to create (hopefully) a uniform, consistent API to quickly build VIs without having to constantly refer to documentation:

  • Begin <> End
  • Create <> Destroy
  • Initialize <> Shutdown
  • Initiate <> Complete
  • Introduce <> Conclude
  • Open <> Close
  • Read <> Write
  • Start <> Stop
  • Send <> Receive

While there is a Channel.lvclass:Close.vi, there is no Channel.lvclass:Open.vi to match it. The Open VI is actually "buried" or implicitly executed within the Channel.lvclass:Create.vi because of the libssh2 implementation of always needing to open and destroy (free) a channel, but optionally needing to close a channel.

I think one possibility is to include a boolean control in the Destroy.vi that is True by default and executes a Close automatically when the channel is destroyed. If the control is False, then the channel is not automatically closed when it is destroyed. This would keep the symmetry of the VIs and naming consistent. However, there is the issue of using the Wait on Close VI for a channel and how to handle the usage? I guess in that case, the boolean control would be False and the user can explicitly call the Close and Wait on Close VI. Both of these VIs would still exist in the public API/palette so there would still be some symmetry problems/inconsistencies.

Also, I believe the underlying libssh2_channel_close function just sends a signal to the remote server that no more data will be sent. So, maybe the simplest option is to rename the VI from Close to "Signal Close", "Terminate" and change the icon. Essentially, change the meaning and naming of the VI to be something that does not have an antonym. The Wait on Close VI would be changed to Wait on Close Signal. I am leaning towards this option at the moment, but I am still not a fan of "Close" begin in the name.

@volks73 volks73 added the enhancement New feature or request label Mar 26, 2020
@volks73 volks73 self-assigned this Mar 26, 2020
@volks73 volks73 added the bug Something isn't working label Mar 26, 2020
@volks73
Copy link
Member Author

volks73 commented Mar 27, 2020

I think I have settled on renaming the Close and Wait on Close VIs as a resolution to this issue, and I am favoring "Send Done" and "Wait on Done" as the new names of the VI, along with changing the icons so the "close" icon is not used. "Done" does not have a complement name within the API.

Adding the "Send" indicates the functionality better. The "Wait on Done" could be also: "Wait on Done Response", "Wait on Done Acknowledge", or "Wait on Done Ack" with "Wait on Done Response" probably being the most appropriate and informative of the functionality.

@volks73
Copy link
Member Author

volks73 commented Mar 28, 2020

Resolved as of bab705a.

I changed the "Close.vi" and "Wait on Closed.vi" VIs to "Done.vi" and "Wait on Done.vi". This was the simplest and most direct resolution to the inconsistency and naming. I also changed the VI icons to use a checkbox glyph, as in "checking off the task".

@volks73
Copy link
Member Author

volks73 commented Mar 28, 2020

Actually, I changed "Done" to "Send Done" because then it matches very nicely with the "Send End-of-File" and the "Wait for End-of-File" VIs, which are in the same row in the Channel palette. This also better indicates the functionality.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant