Closed douglasnorman closed 4 years ago
Bleep bloop!
LabVIEW Diff Robot here with some diffs served up hot for your pull request.
Notice something funny? Help fix me on my GitHub repo.
Overall the changes look great. Just a few nitpicky things:
[x] These should be moved into a subdirectory; possibly a new one for the incoming/outgoing sections? Be sure to move on disk as well.
[x] The text color was changed inadvertently on the Port page. I don't know that we need this divider, but we shouldn't change the text color as part of this PR.
[x] Should the cluster be visible on the port page?
[x] I'd prefer if we used normal strings to persist data - rather than binary ones - unless there is a reason to do otherwise. It makes viewing the system definition as text much easier.
This also goes for the database name, although I'm guessing we'll want to also change that in the template. I'm fine with just fixing the cluster for now.
To start, I agree with Ryan's comments and suggestions. And also nice work on this PR.
Bleep bloop!
LabVIEW Diff Robot here with some diffs served up hot for your pull request.
Notice something funny? Help fix me on my GitHub repo.
Ryan, I made changes to address all your notes. Phil, I started reading New Value terminal in some other VIs, since that was their convention, so I just stuck with it.
TODO: Check the above box with an 'x' indicating you've read and followed CONTRIBUTING.md.
What does this Pull Request accomplish?
It updates Pages and RTMs with additional CAN options. It adds new scripting VIs to support these options. It adds new type defs to support these options.
Why should this Pull Request be merged?
To update System Explorer functionality for the engine work to integrate with.
What testing has been done?
The project has been built and System Explorer has been tested on my dev VM.