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

Fix missing fgen and switch enums. #666

Merged
merged 2 commits into from
Jun 7, 2022

Conversation

reckenro
Copy link
Collaborator

@reckenro reckenro commented Jun 1, 2022

What does this Pull Request accomplish?

FGEN:

  • TriggerWhen enum now referenced by ConfigureDigitalLevelScriptTriggerRequest TriggerWhen param. This lines up with values mentioned in documentation and matches values I found used in .NET implementation.
  • UpdateClockSource enum removed from metadata. Would be used in obsolete NIFGEN_ATTR_UPDATE_CLOCK_SOURCE attribute but we didn't bring that in to grpc-device.

Switch:

  • CabledModuleScanAdvancedBus, CabledModuleTriggerBus, MasterSlaveScanAdvancedBus, MasterSlaveTriggerBus, and TriggerMode enums removed from metadata. These would be referenced by obsolete attributes NISWITCH_ATTR_CABLED_MODULE_SCAN_ADVANCED_BUS, NISWITCH_ATTR_CABLED_MODULE_TRIGGER_BUS, NISWITCH_ATTR_MASTER_SLAVE_SCAN_ADVANCED_BUS. NISWITCH_ATTR_MASTER_SLAVE_TRIGGER_BUS, and NISWITCH_ATTR_TRIGGER_MODE but we didn't bring any of them into grpc-device.

Why should this Pull Request be merged?

Two enums in FGEN and five in Switch weren't being generated in proto.

AB#1991311
AB#1991321

What testing has been done?

Visual inspection of updated proto file. Relying on unit tests, otherwise, since the functions updated aren't referenced in system level tests or examples.

@reckenro reckenro added the binary-breaking Change to proto file that requires client updates label Jun 1, 2022
@reckenro reckenro changed the title Users/reckenro/fgen switch missing enums Fix fgen and switch missing enums. Jun 1, 2022
@reckenro reckenro changed the title Fix fgen and switch missing enums. Fix missing fgen and switch enums. Jun 1, 2022
@reckenro
Copy link
Collaborator Author

reckenro commented Jun 1, 2022

Note: Marked breaking-change because it would break FGEN callers of ConfigureDigitalLevelScriptTrigger who now need to use the TriggerWhen enum for trigger_when argument or set trigger_when_raw instead.

@reckenro
Copy link
Collaborator Author

reckenro commented Jun 7, 2022

@mgumble , I'm going to submit this one since it's pretty straightforward. Feel free to come back to it after you get back.

@reckenro reckenro merged commit d3bf0c5 into ni:main Jun 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
binary-breaking Change to proto file that requires client updates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants