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

Add acyclic frame functionality #54

Merged
merged 6 commits into from
Jul 15, 2022
Merged

Conversation

papowerNI
Copy link
Contributor

@papowerNI papowerNI commented Jul 13, 2022

What does this Pull Request accomplish?

Adds Acyclic frame transmission to the engine in the Tx Acyclic Execution Unit (in a future PR, should rename Tx Acyclic EU to Trigger EU)

Why should this Pull Request be merged?

  • Adds acyclic frames for all transfer types to the engine
  • Updates system tests and unit tests
  • Wraps hardware API calls in a structure to avoid VS crashes when XML contains empty frames
  • Wraps Write to Hardware calls in In Place Structures to have better performance

What testing has been done?

Automated tests have been run, and manually tested the acyclic frames. Tried running with an empty frame of each type, and no crash occurred.
image

@niveristand-diff-bot

This comment was marked as outdated.

@niveristand-diff-bot

This comment was marked as outdated.

@Karl-G1
Copy link
Contributor

Karl-G1 commented Jul 13, 2022

I've updated the system test and asset for acyclic frames, but it's currently failing. RT to BC and BC to RT work, but I am not seeing RT to RT messages contained inside of acyclic frames be sent.

VI Tester Results

image

@niveristand-diff-bot

This comment was marked as outdated.

Copy link
Collaborator

@buckd buckd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Tx Acyclic Write to Hardware - Is this property node access for the triggerable frames going to hit the massive performance problem we found in the Ballard custom device? Should we just use an in place unbundle since we're in the same class?
    image
  • Potential same question as above for Transmitted Messages in the scheduled Write to Hardware.

@papowerNI papowerNI changed the title (**still needs automated tests**) Add acyclic frame functionality Add acyclic frame functionality Jul 14, 2022
@niveristand-diff-bot

This comment was marked as outdated.

@niveristand-diff-bot
Copy link
Collaborator

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.

AIM MIL-STD-1553 Engine.lvlib--Implementation.lvlib--Execution Unit Factory.lvclass--Create Execution Unit.vi.png

capture

AIM MIL-STD-1553 Engine.lvlib--Implementation.lvlib--Execution Unit.lvclass--Get VS Channel Handles and Messages.vi.png

capture

AIM MIL-STD-1553 Engine.lvlib--Implementation.lvlib--Execution Unit.lvclass--Read Module Reference.vi.png

capture

AIM MIL-STD-1553 Engine.lvlib--Implementation.lvlib--Shared Resources Factory.lvclass--Get 1553 Configuration.vi.png

capture

AIM MIL-STD-1553 Engine.lvlib--Implementation.lvlib--Shared Resources.lvclass--Read Bus Controller Message Configurations.vi.png

capture

AIM MIL-STD-1553 Engine.lvlib--Implementation.lvlib--Tx Acyclic Execution Unit.lvclass--Initialize.vi.png

capture

AIM MIL-STD-1553 Engine.lvlib--Implementation.lvlib--Tx Acyclic Execution Unit.lvclass--Populate Triggerable Frames with Messages.vi.png

capture

AIM MIL-STD-1553 Engine.lvlib--Implementation.lvlib--Tx Acyclic Execution Unit.lvclass--Read Triggerable Frames.vi.png

capture

AIM MIL-STD-1553 Engine.lvlib--Implementation.lvlib--Tx Acyclic Execution Unit.lvclass--Tx Acyclic Construct.vi.png

capture

AIM MIL-STD-1553 Engine.lvlib--Implementation.lvlib--Tx Acyclic Execution Unit.lvclass--Write to Hardware.vi.png

capture

AIM MIL-STD-1553 Engine.lvlib--Implementation.lvlib--Tx Execution Unit.lvclass--Write to Hardware.vi.png

capture

AIM MIL-STD-1553 Hardware API.lvlib--Configure Major Frame.vi.png

capture

AIM MIL-STD-1553 Hardware API.lvlib--Configure Minor Frame.vi.png

capture

AIM MIL-STD-1553 Hardware API.lvlib--Configure Simple Frame.vi.png

capture

AIM MIL-STD-1553 Hardware API.lvlib--Send Acyclic Frames.vi.png

capture

AIM MIL-STD-1553 Scripting.lvlib--Find All Acyclic Frames.vi.png

capture

AIM MIL-STD-1553 Scripting.lvlib--Read Terminals and Acyclic Frame Names.vi.png

capture

Create Execution Unit.lvclass--test Get VS Channel Handles and Messages.vi.png

capture

Loopback.lvclass--Execute Acyclic Frame Loopback.vi.png

capture

Loopback.lvclass--test Triggered Acyclic Frame.vi.png

capture

Copy link
Collaborator

@buckd buckd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the skipping of driver calls when transfer IDs aren't found just something we need to do or is there something we need to report back to the user to tell them that something is incorrectly configured?

@Karl-G1
Copy link
Contributor

Karl-G1 commented Jul 15, 2022

Is the skipping of driver calls when transfer IDs aren't found just something we need to do or is there something we need to report back to the user to tell them that something is incorrectly configured?

We should probably put an error in the log file and improve our frame parsing to not persist empty frames/messages into the compiled data.

@papowerNI
Copy link
Contributor Author

papowerNI commented Jul 15, 2022

Is the skipping of driver calls when transfer IDs aren't found just something we need to do or is there something we need to report back to the user to tell them that something is incorrectly configured?

We should probably put an error in the log file and improve our frame parsing to not persist empty frames/messages into the compiled data.

We can do what @Karl-G1 recommended as well, but we also need the case structures because if there is ever an empty array in any of those cases, Veristand crashes.

EDIT: I added an issue (see link below) for this to be addressed in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants