On 04/20/2016 03:40 AM, Lukasz Majewski wrote:
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.
first_usb_dev_port is a different case; it's explicitly data that needs
to be saved from one invocation of the test (on USB port A for example)
to another (on USB port B). The alt setting data should only be used
within a single test invocation.
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?
I would expect the top-level test_dfu() function to read the
configuration values. start_dfu() and dfu_write_read_check() can simply
read them. That way, you wouldn't need the references to be
"test_dfu.xxx" just "xxx". Se how dummy_f is assigned within the outer
function, and the nested functions read that value.
_______________________________________________
U-Boot mailing list
U-Boot@lists.denx.de
http://lists.denx.de/mailman/listinfo/u-boot
