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

Improve usability of get_channel_name in nidigital API #1386

Closed
sbethur opened this issue Apr 7, 2020 · 2 comments · Fixed by #1410
Closed

Improve usability of get_channel_name in nidigital API #1386

sbethur opened this issue Apr 7, 2020 · 2 comments · Fixed by #1410

Comments

@sbethur
Copy link
Contributor

sbethur commented Apr 7, 2020

Description of issue

Currently, there are two very similar methods in the public API:

  1. get_channel_name()
    • Accepts a single channel number as int.
    • Range is [1 : ChannelCount * InstrumentCount]
    • One-based channel input
    • On multi-instruemnt session, returns a string that represents a single fully-qualified channel name e.g. 'PXI1Slot2/0'
    • On single-instruemnt session, returns a string that represents a single channel number e.g. '0'
  2. get_channel_name_from_string()
    • Accepts a str which can be one of the following formats:
      • A comma-separated list - for example, "0,2,3,1"
      • A range using a hyphen - for example, "0-3"
      • A range using a colon - for example, "0:3"
    • Range is [0 : (ChannelCount * InstrumentCount) - 1]
    • Zero-based channel input
    • On multi-instruemnt session, returns a comma-delimited string of fully-qualified channel names e.g. 'PXI1Slot2/0,PXI1Slot5/31'
    • On single-instruemnt session, returns a comma-delimited string of channel numbers e.g. '0,31'

Proposal:

  1. Get rid of get_channel_name()
  2. Rename get_channel_name_from_string() to get_channel_name()
  3. Make the new get_channel_name() use channels repeated capability - This will make this method consistent with rest of the API w.r.t. using channels repeated capability and it will allow the implementation to leverage support for ranges,lists,slices etc. that are available for repeated capabilities.
@marcoskirsch
Copy link
Member

This does not work.
The get_channel_name_from_string method takes in 0-based channel indices. The repeated capability is for channel names, which in multi-instrument session must be fully qualified channel names like "Dev1/4".

We cannot mix the two concepts.

We still need to figure out what to do about these methods though.

@sbethur
Copy link
Contributor Author

sbethur commented Apr 10, 2020

After discussing offline, the proposed solution is:

Any module in nimi-python that needs to provide an API for translating channel indices to names will have only one method - get_channel_names(indices).

  • indices input represents channels numbers. Supported types:
    • str - List (comma delimited)
    • str - Range (using '-' or ':')
    • str - single item
    • int
    • list of str
    • tuple of str
    • range of str
    • slice of str
  • indices will be 0-based (Needs to be clearly documented)
  • Accepted range of indices: [0 : (num_channels_per_instrument * num_instruments) - 1]
  • Return value will be list of str representing fully-qualified channel names like "PXI1Slot2/0,PXI1Slot5/31" on multi-instrument sessions and channel numbers like "0,1,2" on single-instrument sessions

get_channel_name_from_string() and get_channel_name() will be removed from nidigital Python bindings.

Implementation of get_channel_names(indices) in nidigital will use niDigital_GetChannelNameFromString() C entry-point and converters for handling the different types of input and for converting comma-delimited string to list on return value.

NI-Scope C API also contains functions equivalent to get_channel_name_from_string() and get_channel_name(), but niscope Python bindings doesn't. Similar to nidigital, get_channel_names(indices) will be added to niscope - #1402.

nidcpower, nifgen, niswitch Python bindings already have get_channel_name(), whose input is a single 1-based channel index. get_channel_name() will be deprecated and get_channel_names(indices) (with the same behavior as explained above) will be added - #1403.

sbethur added a commit that referenced this issue Apr 13, 2020
… string sequence to comma-delimited string

- Rename convert_repeated_capabilities_from_init() to convert_repeated_capabilities_without_prefix(). Other than its current use for option_string in init(), it will be used for indices in get_channel_names() (#1386) and for initial state parameters in apply_levels_and_timing() (#1391)
- Update _get_ctype_variable_definition_snippet_for_string() to add case for string input with converter and special case for NOT using converter in init function
marcoskirsch pushed a commit that referenced this issue Apr 15, 2020
… string sequence to comma-delimited string (#1404)

* Add codegen support for using converters to convert parameter of type string sequence to comma-delimited string

- Rename convert_repeated_capabilities_from_init() to convert_repeated_capabilities_without_prefix(). Other than its current use for option_string in init(), it will be used for indices in get_channel_names() (#1386) and for initial state parameters in apply_levels_and_timing() (#1391)
- Update _get_ctype_variable_definition_snippet_for_string() to add case for string input with converter and special case for NOT using converter in init function

* Remove the special-case for not using converters in `_init_function`

Without the special-case, converters will be called in `_init_function`
unnecessarily. This is ok since the any kind of special-casing will add
complexity without enough benefit. The redundant call to converters is
not a big deal since it happens only once during session init.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants