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

US1198694: add nidcpower.Session.get_channel_names() #1578

Merged
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 17 additions & 14 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,21 +33,24 @@ All notable changes to this project will be documented in this file.
* #### Removed
* ### `nidcpower` (NI-DCPower)
* #### Added
* `get_channel_names` - [#1588](https://github.com/ni/nimi-python/issues/1588)
* API parity with NI-DCPower 20.7.0 by adding Output Cutoff functionality.
* Properties added:
* `output_cutoff_current_change_limit_high`
* `output_cutoff_current_change_limit_low`
* `output_cutoff_current_measure_limit_high`
* `output_cutoff_current_measure_limit_low`
* `output_cutoff_current_overrange_enabled`
* `output_cutoff_enabled`
* `output_cutoff_voltage_change_limit_high`
* `output_cutoff_voltage_change_limit_low`
* `output_cutoff_voltage_output_limit_high`
* `output_cutoff_voltage_output_limit_low`
* Methods added:
* `clear_latched_output_cutoff_state`
* `query_latched_output_cutoff_state`
* Properties added:
* `output_cutoff_current_change_limit_high`
* `output_cutoff_current_change_limit_low`
* `output_cutoff_current_measure_limit_high`
* `output_cutoff_current_measure_limit_low`
* `output_cutoff_current_overrange_enabled`
* `output_cutoff_enabled`
* `output_cutoff_voltage_change_limit_high`
* `output_cutoff_voltage_change_limit_low`
* `output_cutoff_voltage_output_limit_high`
* `output_cutoff_voltage_output_limit_low`
* Methods added:
* `clear_latched_output_cutoff_state`
* `query_latched_output_cutoff_state`
* #### Changed
* #### Removed
* #### Changed
* #### Removed
* ### `nidigital` (NI-Digital Pattern Driver)
Expand Down
1 change: 1 addition & 0 deletions build/helper/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
from build.helper.helper import get_development_status # noqa: F401
from build.helper.helper import get_numpy_type_for_api_type # noqa: F401
from build.helper.helper import get_python_type_for_api_type # noqa: F401
from build.helper.helper import snakecase_to_camelcase # noqa: F401
from build.helper.helper import sorted_attrs # noqa: F401

from build.helper.metadata_add_all import add_all_metadata # noqa: F401
Expand Down
52 changes: 52 additions & 0 deletions build/helper/helper.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,15 @@ def camelcase_to_snakecase(camelcase_string):
return re.sub('([a-z0-9])([A-Z])', r'\1_\2', s1).lower()


def snakecase_to_camelcase(snakecase_string):
'''Converts a snake_case string to camelCase'''
# https://stackoverflow.com/questions/1175208/elegant-python-function-to-convert-camelcase-to-snake-case
c1 = ''.join(word.title() for word in snakecase_string.split('_'))
if len(c1) > 0:
c1 = c1[0].lower() + c1[1:]
return c1


# TODO(marcoskirsch): not being used
def function_to_method_name(f):
'''Returns an appropriate session method name for a given function'''
Expand Down Expand Up @@ -234,3 +243,46 @@ def test_get_development_status():
assert get_development_status(config) == '5 - Production/Stable'


def test_camelcase_to_snakecase():
camel = 'channelNameList'
expected_snake = 'channel_name_list'
actual_snake = camelcase_to_snakecase(camel)
assert actual_snake == expected_snake

camel = 'channelName'
expected_snake = 'channel_name'
actual_snake = camelcase_to_snakecase(camel)
assert actual_snake == expected_snake

camel = 'channel'
expected_snake = 'channel'
actual_snake = camelcase_to_snakecase(camel)
assert actual_snake == expected_snake

camel = ''
expected_snake = ''
actual_snake = camelcase_to_snakecase(camel)
assert actual_snake == expected_snake


def test_snakecase_to_camelcase():
snake = 'channel_name_list'
expected_camel = 'channelNameList'
actual_camel = snakecase_to_camelcase(snake)
assert actual_camel == expected_camel

snake = 'channel_name'
expected_camel = 'channelName'
actual_camel = snakecase_to_camelcase(snake)
assert actual_camel == expected_camel

snake = 'channel'
expected_camel = 'channel'
actual_camel = snakecase_to_camelcase(snake)
assert actual_camel == expected_camel

snake = ''
expected_camel = ''
actual_camel = snakecase_to_camelcase(snake)
assert actual_camel == expected_camel

40 changes: 28 additions & 12 deletions build/helper/metadata_add_all.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
from .helper import camelcase_to_snakecase
from .helper import get_numpy_type_for_api_type
from .helper import get_python_type_for_api_type
from .helper import snakecase_to_camelcase
from .metadata_filters import filter_codegen_attributes
from .metadata_filters import filter_codegen_functions
from .metadata_find import find_custom_type
Expand Down Expand Up @@ -62,6 +63,9 @@ def _add_python_parameter_name(parameter):
'''Adds a python_name key/value pair to the parameter metadata'''
if 'python_name' not in parameter:
parameter['python_name'] = camelcase_to_snakecase(parameter['name'])
parameter['python_name_override'] = False
else:
parameter['python_name_override'] = True
return parameter


Expand Down Expand Up @@ -271,7 +275,11 @@ def _add_render_in_session_base(f):
def _add_is_repeated_capability(parameter):
Copy link
Member

Choose a reason for hiding this comment

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

This whole method is one big hack. We're inferring if a parameter is a repeated capability based on the name of the parameter; unless explicitly denoted. There are historical reasons for it, but I strongly question it nowadays:

Shouldn't we fix this in metadata? Then (I think) we can remove the inferring code and we can remove all this camel case conversion business.

Here's the thing: converting snakecase_to_camelcase in order to reverse the python name is a bit sketchy. It assumes that python name to C name are always camelCase to snake_case. Which happens to be true right now but doesn't really have to be true.

Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't we fix this in metadata? Then (I think) we can remove the inferring code and we can remove all this camel case conversion business.

Agreed, that the right fix, but it involves retrofitting metadata for all the drivers.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Between the comments in this PR, and now the ones here in this PR, this relatively small user story is beginning to feel like a giant exercise of pulling at threads to see how can much can be unraveled. I won't dispute that there was some technical debt in the code that should probably be addressed at some point -- although I'm not the expert here, I can certainly agree that making assumptions on repeated capabilities based on the name of the parameter doesn't sound great. But I really wonder whether an "inner source" project in which we were asked to help complete the CXP API changes for nidcpower is the best vehicle for solving all this tech debt. After all, we also have a lot of tech debt in TSM that is waiting for attention too. The user story was ostensibly about adding support for instrument-based repeated capabilities for attributes (not even for function parameters). Here we are, almost a month later, and I'm no longer sure what the scope of the user story is any more.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, I stand corrected. This PR isn't even for the repeated capabilities user story (it's been so long, all these PRs are beginning to blur into each other). This one was just for the get_channel_names entry point, which isn't even related to repeated capabilities. It just exposed this issue because I needed to add support in the metadata for python_name in function parameters, which then introduced the need to update the _repeated_capability_parameter_names looksups . So I guess it's more relevant to Marcos' comment than my first comment implied. But we're still pretty far afield from the original user story.

Copy link
Contributor

Choose a reason for hiding this comment

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

Spoke with @luisgomes252 offline and suggested to remove the workaround in _add_is_repeated_capability and add is_repeated_capability = false to nidcpower metadata to prevent the rep cap tip from getting added to the docstring.

The inferring logic will still be present and we should clean it up, not that's outside the scope of this PR.

Even setting is_repeated_capability = false can be done in a separate PR since others are blocked on this PR going in.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Based on the conversation with @sbethur, I've reverted the changes that I had made to use the python_name metadata in determining whether to add the repeated capability. Currently, get_channel_names is incorrectly being generated with metadata, but I'll fix that by updating the dcpower metadata in a subsequent PR so that I can get this one submitted as soon as possible, because a) other PRs are being gated by this one, and b) if I have to grab a new metadata export, that will also pull in a bunch of unrelated changes into this diff. So I'd like to split the PRs, if possible. @marcoskirsch: if you agree, and you have no other objections, please approve this PR when you have a chance.

'''Adds a boolean 'is_repeated_capability' to the parameter metadata by inferring it from its name, if not previously populated.'''
if 'is_repeated_capability' not in parameter:
if parameter['name'] in _repeated_capability_parameter_names:
if parameter['python_name_override']:
is_rep = snakecase_to_camelcase(parameter['python_name']) in _repeated_capability_parameter_names
else:
is_rep = parameter['name'] in _repeated_capability_parameter_names
if is_rep:
parameter['is_repeated_capability'] = True
parameter['repeated_capability_type'] = 'channels'
else:
Expand Down Expand Up @@ -691,6 +699,7 @@ def _compare_dicts(actual, expected):
'direction': 'in',
'enum': None,
'name': 'channelName',
'python_name': 'name',
'type': 'ViString',
'documentation': {
'description': 'The channel to call this on.',
Expand Down Expand Up @@ -793,10 +802,9 @@ def _compare_dicts(actual, expected):
'documentation': {
'description': 'Performs a foo, and performs it well.'
},
'has_repeated_capability': True,
'repeated_capability_type': 'channels',
'has_repeated_capability': False,
'is_error_handling': False,
'render_in_session_base': True,
'render_in_session_base': False,
'method_templates': [{'session_filename': '/cool_template', 'documentation_filename': '/cool_template', 'method_python_name_suffix': '', }, ],
'parameters': [
{
Expand Down Expand Up @@ -830,17 +838,17 @@ def _compare_dicts(actual, expected):
'library_method_call_snippet': 'vi_ctype',
'use_in_python_api': True,
'python_name_or_default_for_init': 'vi',
'python_name_override': False,
},
{
'ctypes_type': 'ViString',
'ctypes_variable_name': 'channel_name_ctype',
'ctypes_variable_name': 'name_ctype',
'ctypes_type_library_call': 'ctypes.POINTER(ViChar)',
'direction': 'in',
'documentation': {
'description': 'The channel to call this on.'
},
'is_repeated_capability': True,
'repeated_capability_type': 'channels',
'is_repeated_capability': False,
'is_session_handle': False,
'enum': None,
'numpy': False,
Expand All @@ -852,14 +860,15 @@ def _compare_dicts(actual, expected):
'use_list': False,
'is_string': True,
'name': 'channelName',
'python_name': 'channel_name',
'python_name_with_default': 'channel_name',
'python_name_with_doc_default': 'channel_name',
'python_name': 'name',
'python_name_with_default': 'name',
'python_name_with_doc_default': 'name',
'size': {'mechanism': 'fixed', 'value': 1},
'type': 'ViString',
'library_method_call_snippet': 'channel_name_ctype',
'library_method_call_snippet': 'name_ctype',
'use_in_python_api': True,
'python_name_or_default_for_init': 'channel_name',
'python_name_or_default_for_init': 'name',
'python_name_override': True,
},
{
'ctypes_type': 'ViInt32',
Expand Down Expand Up @@ -892,6 +901,7 @@ def _compare_dicts(actual, expected):
'library_method_call_snippet': 'pin_data_buffer_size_ctype',
'use_in_python_api': False,
'python_name_or_default_for_init': 'pin_data_buffer_size',
'python_name_override': False,
},
{
'ctypes_type': 'ViInt32',
Expand Down Expand Up @@ -924,6 +934,7 @@ def _compare_dicts(actual, expected):
'library_method_call_snippet': 'None if actual_num_pin_data_ctype is None else (ctypes.pointer(actual_num_pin_data_ctype))',
'use_in_python_api': False,
'python_name_or_default_for_init': 'actual_num_pin_data',
'python_name_override': False,
},
{
'ctypes_type': 'ViUInt8',
Expand Down Expand Up @@ -958,6 +969,7 @@ def _compare_dicts(actual, expected):
'library_method_call_snippet': 'expected_pin_states_ctype',
'use_in_python_api': True,
'python_name_or_default_for_init': 'expected_pin_states',
'python_name_override': False,
},
],
'python_name': 'make_a_foo',
Expand Down Expand Up @@ -1000,6 +1012,7 @@ def _compare_dicts(actual, expected):
'library_method_call_snippet': 'vi_ctype',
'use_in_python_api': True,
'python_name_or_default_for_init': 'vi',
'python_name_override': False,
},
{
'direction': 'out',
Expand Down Expand Up @@ -1032,6 +1045,7 @@ def _compare_dicts(actual, expected):
'library_method_call_snippet': 'status_ctype',
'use_in_python_api': True,
'python_name_or_default_for_init': 'status',
'python_name_override': False,
},
{
'ctypes_type': 'ViInt32',
Expand Down Expand Up @@ -1064,6 +1078,7 @@ def _compare_dicts(actual, expected):
'library_method_call_snippet': 'data_buffer_size_ctype',
'use_in_python_api': False,
'python_name_or_default_for_init': 'data_buffer_size',
'python_name_override': False,
},
{
'ctypes_type': 'ViUInt32',
Expand Down Expand Up @@ -1097,6 +1112,7 @@ def _compare_dicts(actual, expected):
'library_method_call_snippet': 'data_ctype',
'use_in_python_api': True,
'python_name_or_default_for_init': 'data',
'python_name_override': False,
},
],
'documentation': {
Expand Down
39 changes: 39 additions & 0 deletions docs/nidcpower/class.rst
Original file line number Diff line number Diff line change
Expand Up @@ -858,6 +858,45 @@ get_channel_name



get_channel_names
-----------------

.. py:currentmodule:: nidcpower.Session

.. py:method:: get_channel_names(indices)

Returns a list of channel names for given channel indices.





:param indices:


Index list for the channels in the session. Valid values are from zero to the total number of channels in the session minus one. The index string 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 "

You can combine comma-separated lists and ranges that use a hyphen or colon. Both out-of-order and repeated indices are supported ("2,3,0," "1,2,2,3"). White space characters, including spaces, tabs, feeds, and carriage returns, are allowed between characters. Ranges can be incrementing or decrementing.




:type indices: basic sequence types or str or int

:rtype: list of str
:return:


The channel name(s) at the specified indices.





get_ext_cal_last_date_and_time
------------------------------

Expand Down
9 changes: 9 additions & 0 deletions generated/nidcpower/nidcpower/_library.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ def __init__(self, ctypes_library):
self.niDCPower_GetAttributeViReal64_cfunc = None
self.niDCPower_GetAttributeViString_cfunc = None
self.niDCPower_GetChannelName_cfunc = None
self.niDCPower_GetChannelNameFromString_cfunc = None
self.niDCPower_GetError_cfunc = None
self.niDCPower_GetExtCalLastDateAndTime_cfunc = None
self.niDCPower_GetExtCalLastTemp_cfunc = None
Expand Down Expand Up @@ -225,6 +226,14 @@ def niDCPower_GetChannelName(self, vi, index, buffer_size, channel_name): # noq
self.niDCPower_GetChannelName_cfunc.restype = ViStatus # noqa: F405
return self.niDCPower_GetChannelName_cfunc(vi, index, buffer_size, channel_name)

def niDCPower_GetChannelNameFromString(self, vi, indices, buffer_size, names): # noqa: N802
with self._func_lock:
if self.niDCPower_GetChannelNameFromString_cfunc is None:
self.niDCPower_GetChannelNameFromString_cfunc = self._get_library_function('niDCPower_GetChannelNameFromString')
self.niDCPower_GetChannelNameFromString_cfunc.argtypes = [ViSession, ctypes.POINTER(ViChar), ViInt32, ctypes.POINTER(ViChar)] # noqa: F405
self.niDCPower_GetChannelNameFromString_cfunc.restype = ViStatus # noqa: F405
return self.niDCPower_GetChannelNameFromString_cfunc(vi, indices, buffer_size, names)

def niDCPower_GetError(self, vi, code, buffer_size, description): # noqa: N802
with self._func_lock:
if self.niDCPower_GetError_cfunc is None:
Expand Down
32 changes: 32 additions & 0 deletions generated/nidcpower/nidcpower/session.py
Original file line number Diff line number Diff line change
Expand Up @@ -5185,6 +5185,38 @@ def get_channel_name(self, index):
errors.handle_error(self, error_code, ignore_warnings=False, is_error_handling=False)
return channel_name_ctype.value.decode(self._encoding)

@ivi_synchronized
def get_channel_names(self, indices):
r'''get_channel_names

Returns a list of channel names for given channel indices.

Args:
indices (basic sequence types or str or int): Index list for the channels in the session. Valid values are from zero to the total number of channels in the session minus one. The index string 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 "

You can combine comma-separated lists and ranges that use a hyphen or colon. Both out-of-order and repeated indices are supported ("2,3,0," "1,2,2,3"). White space characters, including spaces, tabs, feeds, and carriage returns, are allowed between characters. Ranges can be incrementing or decrementing.


Returns:
names (list of str): The channel name(s) at the specified indices.

'''
vi_ctype = _visatype.ViSession(self._vi) # case S110
indices_ctype = ctypes.create_string_buffer(_converters.convert_repeated_capabilities_without_prefix(indices).encode(self._encoding)) # case C040
buffer_size_ctype = _visatype.ViInt32() # case S170
names_ctype = None # case C050
error_code = self._library.niDCPower_GetChannelNameFromString(vi_ctype, indices_ctype, buffer_size_ctype, names_ctype)
errors.handle_error(self, error_code, ignore_warnings=True, is_error_handling=False)
buffer_size_ctype = _visatype.ViInt32(error_code) # case S180
names_ctype = (_visatype.ViChar * buffer_size_ctype.value)() # case C060
error_code = self._library.niDCPower_GetChannelNameFromString(vi_ctype, indices_ctype, buffer_size_ctype, names_ctype)
errors.handle_error(self, error_code, ignore_warnings=False, is_error_handling=False)
return _converters.convert_comma_separated_string_to_list(names_ctype.value.decode(self._encoding))

@ivi_synchronized
def _get_ext_cal_last_date_and_time(self):
r'''_get_ext_cal_last_date_and_time
Expand Down
Loading