Add default value to nislsc python api by zoechanzy · Pull Request #67 · ni/nislsc-python

zoechanzy · 2026-08-06T05:06:31Z

What does this Pull Request accomplish?

Update nislscapi_full.json
Update function_helper.py and templates files to consume default value from nislscapi_full.json
Added default value of None for the library parameter
Regenerate generated python modules files
Refactor parameter ordering in unit tests and examples

Why should this Pull Request be merged?

nislscapi.json is updated to have default values. Function helper and template files should be updated so that the default value can be used in code-gen.

What testing has been done?

Passed unit tests

Related work item:
USER STORY 3221873

--------- Signed-off-by: zoechanzy <zoe.chan@emerson.com>

* Update function_helpers.py and templates files to consume default value field from nislscapi_full.json * Refactor the order of parameters to avoid python TypeError: positional argument follows keyword argument error * Regenerate generated Python module files --------- Signed-off-by: zoechanzy <zoe.chan@emerson.com>

ni-github-admins · 2026-08-06T05:06:47Z

✅ Snyk checks have passed. No issues have been found so far.

Status	Scan Engine	Critical	High	Medium	Low	Total (0)
✅	Code Security	0	0	0	0	0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

github-actions · 2026-08-06T05:07:31Z

Test Results

14 files ±0 14 suites ±0 15m 2s ⏱️ -41s
224 tests ±0 224 ✅ ±0 0 💤 ±0 0 ❌ ±0
3 136 runs ±0 3 136 ✅ ±0 0 💤 ±0 0 ❌ ±0

Results for commit e906a73. ± Comparison against base commit 0903e49.

♻️ This comment has been updated with latest results.

* Added default value of None for the library parameter * Regenerate generated Python module files --------- Signed-off-by: zoechanzy <zoe.chan@emerson.com>

* Refactor parameter ordering in unit tests --------- Signed-off-by: zoechanzy <zoe.chan@emerson.com>

Copilot

Pull request overview

This PR updates the codegen metadata and templates so generated NI-SLSC Python APIs can expose parameter default values (notably library=None, timeouts, and “default resource” sentinels), and regenerates the affected modules and tests accordingly.

Changes:

Added default fields to nislscapi_full.json for various parameters (timeouts, default resources).
Updated codegen helpers/templates to emit defaults in generated Python signatures (and to reorder required vs defaulted parameters).
Regenerated generated/nislsc/session.py and updated unit tests/examples to match the new call signatures.

Reviewed changes

Copilot reviewed 12 out of 13 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
tests/unit/test_session.py	Updates test calls to match reordered/generated Session method signatures and new keyword usage.
tests/unit/conftest.py	Updates the session fixture to use new `initialize_session_with_devices` signature/keywords.
src/codegen/utilities/function_helpers.py	Adds “include defaults” support, default formatting helpers, and signature reordering logic.
src/codegen/templates/utils.py.mako	Enables default emission for module-level generated functions.
src/codegen/templates/session.py.mako	Enables default emission for generated `Session` methods/classmethods.
src/codegen/templates/property.py.mako	Enables default emission for generated `PropertyReference` APIs.
src/codegen/templates/library.py.mako	Enables default emission for generated `Library` APIs.
src/codegen/templates/command.py.mako	Enables default emission for generated `CommandReference` APIs.
src/codegen/metadata/nislscapi_full.json	Adds default values in metadata (timeouts, `$Default*` resource sentinels).
generated/nislsc/session.py	Regenerated Session API with defaults and reordered parameters.
examples/show_command_and_property_tree.py	Updates example calls to match new Session method parameter order.
examples/reset_device.py	Updates example initialization call to use new keyword/default patterns.
examples/check_chassis_battery.py	Updates example property calls to match new parameter ordering/defaults.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

tjying95 · 2026-08-10T03:05:29Z

            )
            physical_channel_properties = session.get_physical_channel_property_string_array(
-                physical_channel, PhysicalChannelProperty.PROPERTIES
+                PhysicalChannelProperty.PROPERTIES, physical_channel


@bkeryan With $DefaultPhysChans and $DefaultDevices set, we have to put the physical_channel and device_name to the back. Feels a bit weird (I am still fine with it).
Alternative is having positional arguments default to None or "", then we error out if not set. But that will probably mess the docstring and IntelliSense. Thoughts?

I agree it seems a little weird.

The approach used by other NI driver APIs for Python is to have a collection of objects (e.g. nidaqmx channels or MI repeated capabilities) that returns proxy objects with the 1st parameter bound (e.g. device).

# session.devices["Mod1"] returns a Device object that is bound to Mod1 session.devices["Mod1"].set_property_string_array(XYZ, ["a","b","c"]) # passes Mod1 for 1st parameter

SLSC is not an IVI driver, but the new Python IVI spec also uses this approach.

The collection can expose a separate property that returns a proxy that is bound to $DeviceDevices:

# session.devices.default returns a Device object that is bound to $DefaultDevices session.devices.default.set_property_string_array(XYZ, ["a","b","c"]) # passes $DefaultDevices for 1st parameter

That is a bigger API change, though. Working within the current API, I think it would be ok for (get|set)_device_xyz(), (get|set)_phys_chan_xyz(), (get|set)_nvmem_bytes(), etc. to not have a default for the "active context" (1st parameter that specifies which device/physchan/nvmem you are accessing).

If we remove the default "active context" parameter, users can still write session.set_device_property_string_array("$DefaultDevices", XYZ, ["a","b","c"]).

Also, I think that defining constants for the resource aliases like DEFAULT_DEVICES_ALIAS = "$DefaultDevices" would make them more discoverable.

(get|set)_device_xyz(), (get|set)_phys_chan_xyz(), (get|set)_nvmem_bytes(), etc. to not have a default for the "active context" (1st parameter that specifies which device/physchan/nvmem you are accessing)

I think there is a few ways to address it:
Method1:
drop devices, physChans or nvmemAreas default value of (get|set)_device_xyz(), (get|set)_phys_chan_xyz(), (get|set)_nvmem_bytes(), etc from nislscapi.json directly

Method2:
Remain devices, physChans or nvmemAreas default value of (get|set)_device_xyz(), (get|set)_phys_chan_xyz(), (get|set)_nvmem_bytes(), etc in nislscapi.json. Add addtional function in function_helpers.py to ignore the corresponding default values in the setter / getter, etc

I think method 1 is easier and straight-forward.

tjying95 · 2026-08-11T06:00:17Z

Perform the tests on actual hardware and make sure the examples still work. Also try writing a simple python script to test if the default parameter is working as intended, especially the $DefaultDevices and $DefaultPhysChans.

* Refactor parameter ordering in examples to match new defaults * Remove library,connection_timeout and reservation_timeout parameters from examples since they have default values. --------- Signed-off-by: zoechanzy <zoe.chan@emerson.com>

…types --------- Signed-off-by: zoechanzy <zoe.chan@emerson.com>

bkeryan · 2026-08-11T17:44:11Z


    @classmethod
-    def initialize_session_with_physical_channels(cls, library: Library | None, physical_channel_names: str, connection_timeout: float, reservation_access: ReservationAccess, reservation_group: str, reservation_timeout: float) -> Self:
+    def initialize_session_with_physical_channels(cls, physical_channel_names: str, reservation_access: ReservationAccess, reservation_group: str, library: Library | None = None, connection_timeout: float = -1.0, reservation_timeout: float = -1.0) -> Self:


The init methods should have access=read/write group="" by default. Clients should not have to specify reservation parameters unless they want to share devices/channels between sessions.

Thank you so much. I will update reservationAccess and reservationGroup parameters of functions like InitializeSessionWithDevices to have the default value in the nislscapi.json.

bkeryan · 2026-08-11T17:46:25Z

        return chassis_name

-    def reserve_devices(self, device_names: str, reservation_access: ReservationAccess, reservation_group: str, reservation_timeout: float) -> None:
+    def reserve_devices(self, reservation_access: ReservationAccess, reservation_group: str, device_names: str = '$DefaultDevices', reservation_timeout: float = -1.0) -> None:


Ditto for reserve: reservation parameters should have defaults.

session.reserve() should reserve the session's default devices, and session.reserve("Mod1") should reserve Mod1.

bkeryan · 2026-08-11T17:53:48Z

            )
            physical_channel_properties = session.get_physical_channel_property_string_array(
-                physical_channel, PhysicalChannelProperty.PROPERTIES
+                PhysicalChannelProperty.PROPERTIES, physical_channel


I agree it seems a little weird.

The approach used by other NI driver APIs for Python is to have a collection of objects (e.g. nidaqmx channels or MI repeated capabilities) that returns proxy objects with the 1st parameter bound (e.g. device).

# session.devices["Mod1"] returns a Device object that is bound to Mod1 session.devices["Mod1"].set_property_string_array(XYZ, ["a","b","c"]) # passes Mod1 for 1st parameter

SLSC is not an IVI driver, but the new Python IVI spec also uses this approach.

The collection can expose a separate property that returns a proxy that is bound to $DeviceDevices:

# session.devices.default returns a Device object that is bound to $DefaultDevices session.devices.default.set_property_string_array(XYZ, ["a","b","c"]) # passes $DefaultDevices for 1st parameter

That is a bigger API change, though. Working within the current API, I think it would be ok for (get|set)_device_xyz(), (get|set)_phys_chan_xyz(), (get|set)_nvmem_bytes(), etc. to not have a default for the "active context" (1st parameter that specifies which device/physchan/nvmem you are accessing).

bkeryan · 2026-08-11T18:01:39Z

+        connection_timeout=-1.0,
+        reservation_timeout=-1.0,


Omit the timeouts when they are the same as the default.

bkeryan · 2026-08-11T18:01:44Z

+        ReservationAccess.NONE,
+        "",
+        library=library,
+        connection_timeout=-1.0,


Omit the timeouts when they are the same as the default.

bkeryan · 2026-08-11T18:02:18Z

+        "Area1",
+        ReservationAccess.NONE,
+        "",
+        library=None,


Omit library=None when it is the same as the default.

* Add * in get_function_parameter_list to make the Booleans keyword-only * Remove argument from get_function_parameter_list if the argument have default value. * Use named parameters for get_function_parameter_list to avoid ambiguity

* Add CONNECTED_DEVICES_ALIAS constant * Add RESERVED_DEVICES_ALIAS constant * Add DEFAULT_NVMEM_AREAS_ALIAS constant * Add DEFAULT_PHYS_CHANS_ALIAS constant * Add RESERVED_DEVICES_ALIAS constant * Add SESSION_ALIAS constant * Add SYSTEM_ALIAS constant --------- Signed-off-by: zoechanzy <zoe.chan@emerson.com>

zoechanzy added 2 commits August 6, 2026 09:47

default value: Update nislscapi_full.json

3bdf708

--------- Signed-off-by: zoechanzy <zoe.chan@emerson.com>

zoechanzy force-pushed the users/zchan/nislsc_default_value branch 3 times, most recently from af3371e to 6bb49f5 Compare August 6, 2026 05:54

default value: Add default value for library parameter

646ecac

* Added default value of None for the library parameter * Regenerate generated Python module files --------- Signed-off-by: zoechanzy <zoe.chan@emerson.com>

zoechanzy force-pushed the users/zchan/nislsc_default_value branch from 6bb49f5 to 646ecac Compare August 6, 2026 06:11

zoechanzy changed the title ~~Users/zchan/nislsc default value~~ Add default value to nislsc python api Aug 7, 2026

default value: Update unit tests for new defaults

7dd42d6

* Refactor parameter ordering in unit tests --------- Signed-off-by: zoechanzy <zoe.chan@emerson.com>

zoechanzy force-pushed the users/zchan/nislsc_default_value branch from 851de41 to fdd3ed9 Compare August 7, 2026 03:12

zoechanzy requested a lite review from Copilot August 7, 2026 03:13

Copilot started reviewing on behalf of zoechanzy August 7, 2026 03:14 View session

Copilot AI reviewed Aug 7, 2026

View reviewed changes

Comment thread src/codegen/utilities/function_helpers.py

Comment thread src/codegen/utilities/function_helpers.py

zoechanzy marked this pull request as ready for review August 7, 2026 05:36

zoechanzy requested review from bkeryan, kkang-NI and tjying95 as code owners August 7, 2026 05:36

tjying95 reviewed Aug 10, 2026

View reviewed changes

Comment thread examples/show_command_and_property_tree.py Outdated

tjying95 reviewed Aug 10, 2026

View reviewed changes

Comment thread examples/show_command_and_property_tree.py Outdated

tjying95 requested a review from junliang-tan August 11, 2026 06:01

zoechanzy added 2 commits August 11, 2026 16:06

default value: Update examples for new defaults

14f3ae7

* Refactor parameter ordering in examples to match new defaults * Remove library,connection_timeout and reservation_timeout parameters from examples since they have default values. --------- Signed-off-by: zoechanzy <zoe.chan@emerson.com>

fix: convert integer defaults to float for double and TimeoutSeconds …

30845a7

…types --------- Signed-off-by: zoechanzy <zoe.chan@emerson.com>

zoechanzy force-pushed the users/zchan/nislsc_default_value branch from 0bf3a56 to 30845a7 Compare August 11, 2026 08:09

bkeryan requested changes Aug 11, 2026

View reviewed changes

bkeryan reviewed Aug 11, 2026

View reviewed changes

Comment thread examples/check_chassis_battery.py

Uh oh!

Conversation

zoechanzy commented Aug 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this Pull Request accomplish?

Why should this Pull Request be merged?

What testing has been done?

Uh oh!

ni-github-admins commented Aug 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Snyk checks have passed. No issues have been found so far.

Uh oh!

github-actions Bot commented Aug 6, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

zoechanzy Aug 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

tjying95 commented Aug 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

zoechanzy commented Aug 6, 2026 •

edited

Loading

ni-github-admins commented Aug 6, 2026 •

edited

Loading

github-actions Bot commented Aug 6, 2026 •

edited

Loading

zoechanzy Aug 13, 2026 •

edited

Loading

tjying95 commented Aug 11, 2026 •

edited

Loading