From mboxrd@z Thu Jan 1 00:00:00 1970 From: Lukasz Majewski Date: Wed, 20 Apr 2016 11:40:17 +0200 Subject: [U-Boot] [PATCH v2 3/3] tests: py: dfu: Provide functionality to set test and dummy files alt settings In-Reply-To: <57165BF0.8040607@wwwdotorg.org> References: <1460130291-24223-1-git-send-email-l.majewski@samsung.com> <1461081084-16649-1-git-send-email-l.majewski@samsung.com> <1461081084-16649-3-git-send-email-l.majewski@samsung.com> <57165BF0.8040607@wwwdotorg.org> Message-ID: <20160420114017.3d1df243@amdc2363> List-Id: MIME-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit To: u-boot@lists.denx.de Hi Stephen, > On 04/19/2016 09:51 AM, Lukasz Majewski wrote: > > After concatenation of "dfu_alt_info" variable from "dfu_alt_boot" > > and "dfu_alt_system" it may happen that test and dummy files alt > > settings are different than default 0 and 1. > > > > This patch provides the ability to set different values for them. > > > @@ -122,6 +139,8 @@ def test_dfu(u_boot_console, env__usb_dev_port, > > env__dfu_config): Returns: > > Nothing. > > """ > > + global alt_setting_test_file > > + global alt_setting_dummy_file > > There should be a blank line after the """ line. Although per the > comments below, you can simply drop this part of the diff completely. > > > @@ -132,6 +151,9 @@ def test_dfu(u_boot_console, env__usb_dev_port, > > env__dfu_config): u_boot_console.log.action( > > 'Starting long-running U-Boot dfu shell command') > > > > + alt_setting_test_file = > > env__dfu_config.get('alt_id_test_file', '0') > > + alt_setting_dummy_file = > > env__dfu_config.get('alt_id_dummy_file', '1') > > This always over-writes alt_setting_test_file, and changes the type > from integer (as specified by the current global assignment added in > patch 1) to string. You may as well simply remove the "global" lines > added in this patch, and the global assignment, since this patch > always assigns a value to those variables. > > Since the variable always contains a string now, you can remove the > str() call from run_dfu_util()'s assignment to cmd[]. > Frankly I'm a bit confused now. I'm not the python expert, but v2 design seems logical to me: test_dfu.py Python module (outermost scope): alt_setting_test_file = 0 alt_setting_dummy_file = 1 def test_dfu: def start_dfu(): global alt_setting_test_file global alt_setting_dummy_file alt_setting_test_file=env__dfu_config.get('alt_id_test_file','0') alt_setting_dummy_file=env__dfu_config.get('alt_id_dummy_file','1') def dfu_write_read_check(): ... dfu_write(alt_setting_test_file, test_f.abs_fn) ... dfu_write(alt_setting_dummy_file, dummy_f.abs_fn) ... dfu_write(alt_setting_test_file, dummy_f.abs_fn) So I'm setting up alt_setting_{test|dummy}_file global variables at start_dfu() function. Afterwards, I reuse them at dfu_write_read_check(). Such approach is similar to one used for "first_usb_dev_port" global variable in the same file. If I remove "global" from alt_setting_{test|dummy}_file declaration then I will set them only to be valid at start_dfu() function scope. Hence at dfu_write_read_check() I will use those variables set to 0 and 1 respectively. One possible solution that I've found is to use "function attributes": def test_dfu: test_dfu.alt_setting_test_file test_dfu.alt_setting_dummy_file def start_dfu(): test_dfu.alt_setting_test_file=env__dfu_config.get('alt_id_test_file','0') test_dfu.alt_setting_dummy_file=env__dfu_config.get('alt_id_dummy_file','1') def dfu_write_read_check(): ... dfu_write(test_dfu.alt_setting_test_file, test_f.abs_fn) ... Stephen, is the above solution acceptable for you? -- Best regards, Lukasz Majewski Samsung R&D Institute Poland (SRPOL) | Linux Platform Group