Skip to content

Remove export signal#848

Merged
marcoskirsch merged 6 commits intomasterfrom
bug828/remove_export_signal
May 8, 2018
Merged

Remove export signal#848
marcoskirsch merged 6 commits intomasterfrom
bug828/remove_export_signal

Conversation

@texasaggie97-zz
Copy link
Contributor

  • This contribution adheres to CONTRIBUTING.md.
  • I've updated CHANGELOG.md if applicable.
  • I've addedremoved tests applicable for this pull request

What does this Pull Request accomplish?

  • Remove export_signal from nidcpower, nifgen, & niscope
    • We want clients to "Pull" the signal from the device instead of exporting. I.e. The scope's trigger source would be 'fgen5433\sometrigger'
    • export_signal() does not provide all the functionality that properties do

List issues fixed by this Pull Request below, if any.

What testing has been done?

  • Unit
  • System

TIME_HISTOGRAM_NEW_HITS = 3011


class ExportableSignals(Enum):
Copy link
Member

Choose a reason for hiding this comment

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

In NI-FGEN this was called Signal. Needless inconsistency. Glad it's gone.

@marcoskirsch marcoskirsch merged commit 576620c into master May 8, 2018
@marcoskirsch marcoskirsch deleted the bug828/remove_export_signal branch May 8, 2018 18:30
@mitchellkelley
Copy link

@marcoskirsch Can you give an example for what the replacement of the Export Signal function would look like using the properties as mentioned at the top. For example, how would you export a trigger to PFI 0 on a FGEN card?

@marcoskirsch
Copy link
Member

For example, how would you export a trigger to PFI 0 on a FGEN card?

my_session.exported_start_trigger_output_terminal = "PFI0"

@gfisher-NI
Copy link
Contributor

@marcoskirsch @mitchellkelley I think this is a case where we may want to have something in the documentation explaining how to do this configuration, as this is a place where the Python API differs from the C API (where niFgen_ExportSignal exists). Usually, if documentation doesn't exist for something in Python, users (and supporters) can reference the C or LabVIEW API and generally figure out what they need to do, but that's not the case here.

@sbethur
Copy link
Contributor

sbethur commented Dec 1, 2020

I think this is a case where we may want to have something in the documentation explaining how to do this configuration, as this is a place where the Python API differs from the C API (where niFgen_ExportSignal exists). Usually, if documentation doesn't exist for something in Python, users (and supporters) can reference the C or LabVIEW API and generally figure out what they need to do, but that's not the case here.

@gfisher-NI, can you please open an issue on GitHub for this.

@gfisher-NI
Copy link
Contributor

@sbethur Done!

@gfisher-NI
Copy link
Contributor

gfisher-NI commented Dec 16, 2020

@marcoskirsch @sbethur @mitchellkelley I'm trying to put together an example of this, and where I am stuck is that there is no indication anywhere of what values digital_edge_start_trigger_source and exported_start_trigger_output_terminal can take. I would expect there to be an enum for it and there isn't.

Based on both the existing documentation and what was stated above, the following code should work and does not:

  with nifgen.Session('PXI1Slot12', [0,1]) as dev1, nifgen.Session('PXI1Slot16', [0,1]) as dev2:
      dev_list = [dev1, dev2]
      for dev in dev_list:
          dev.start_trigger_type = nifgen.StartTriggerType.TRIG_NONE
          dev.output_mode = nifgen.OutputMode.FUNC
          dev.configure_standard_waveform(waveform=nifgen.Waveform.SINE, amplitude=1.0, frequency=100000, dc_offset=0.0, start_phase=0.0)
          
      dev1.start_trigger_type = nifgen.StartTriggerType.SOFTWARE_EDGE
      dev1.exported_start_trigger_output_terminal = 'PXI_Trig0'
      
      dev2.start_trigger_type = nifgen.StartTriggerType.DIGITAL_EDGE
      dev2.digital_edge_start_trigger_edge = nifgen.StartTriggerDigitalEdgeEdge.RISING
      dev2.digital_edge_start_trigger_source = 'PXI_Trig0'
      
      with dev2.initiate():   
          with dev1.initiate():
              dev1.send_software_edge_trigger(nifgen.Trigger.START)
              time.sleep(1)
  
  

@marcoskirsch
Copy link
Member

@gfisher-NI, we made the explicit the decision to not include constants in the API for terminals on the same instrument. The optimal way of using our APIs is to leverage NI's routing subsystem and "pull" signals from other instruments. In other ADEs we do declare constants, but this is mostly a carryover practice from before NI's routing subsystem (~2002).

See https://github.com/ni/nimi-python/wiki/API-design-guidelines#encourage-use-of-cross-device-trigger-routing

Unfortunately, it is not obvious to customers what the valid strings are for a given system configuration. Some instruments have a Routing tab in MAX and this can be referrenced. Many others don't. This is a widespread documentation gap throughout NI driver software.

@gfisher-NI
Copy link
Contributor

gfisher-NI commented Jan 5, 2021

@marcoskirsch Okay, that does make sense, and also pointed me at the mistake in my code. This code works:

with nifgen.Session('PXI1Slot12', [0,1]) as dev1, nifgen.Session('PXI1Slot16', [0,1]) as dev2:
    dev_list = [dev1, dev2]
    for dev in dev_list:
        dev.start_trigger_type = nifgen.StartTriggerType.TRIG_NONE
        dev.output_mode = nifgen.OutputMode.FUNC
        dev.configure_standard_waveform(waveform=nifgen.Waveform.SINE, amplitude=1.0, frequency=100000, dc_offset=0.0, start_phase=0.0)
        
    dev1.start_trigger_type = nifgen.StartTriggerType.SOFTWARE_EDGE
    
    dev2.start_trigger_type = nifgen.StartTriggerType.DIGITAL_EDGE
    dev2.digital_edge_start_trigger_edge = nifgen.StartTriggerDigitalEdgeEdge.RISING
    dev2.digital_edge_start_trigger_source = '/PXI1Slot12/0/StartTrigger'
    
    with dev2.initiate():   
        with dev1.initiate():
            dev1.send_software_edge_trigger(nifgen.Trigger.START)
            time.sleep(1)

@mitchellkelley

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

export_signal() functions are redundant and incomplete

6 participants