Usb msd tests #9444

maciejbocianski · 2019-01-21T14:53:05Z

Description

Test for USB MSD calss
Tested on Windows and Linux. On Mac should also work
Supported targets see TESTS/usb_device/msd/mbed_app.json
Targets with small RAM (less then 70kB) and without flash memory (e.g ARCH_PRO) require ci-sheild + SD card to enable testing

TODO:

update README.md

Known issues:

sometimes fs get corrupted while mounting on Windows machine
sometimes test hungs due to that greentea_parse_kv is preempted by usb msd communication task (it doesn't get message from host despite is was send and is visible on serial console)

Pull request type

[ ] Fix
[ ] Refactor
[ ] Target update
[ ] Functionality change
[ ] Docs update
[X] Test update
[ ] Breaking change

Reviewers

@c1728p9

0xc0170 · 2019-01-21T15:33:51Z

Please review astyle travis job

ciarmcom · 2019-01-21T16:00:41Z

@maciejbocianski, thank you for your changes.
@c1728p9 @ARMmbed/mbed-os-maintainers please review.

maciejbocianski · 2019-01-21T20:25:55Z

astyle fixed

0xc0170 · 2019-01-22T13:58:50Z

TESTS/host_tests/pyusb_msd.py

@@ -0,0 +1,242 @@
+"""
+mbed SDK


can you use updated license header

/* * Copyright (c) 2019, Arm Limited and affiliates. * SPDX-License-Identifier: Apache-2.0 * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. * You may obtain a copy of the License at * * http://www.apache.org/licenses/LICENSE-2.0 * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. * See the License for the specific language governing permissions and * limitations under the License. */

license headers fixed

c1728p9

This looks great so far. I left some minor feedback. Once the stability problems you mentioned above are fixed then this will be ready to go.

c1728p9 · 2019-01-23T18:14:02Z

TESTS/usb_device/msd/mbed_app.json

@@ -0,0 +1,39 @@
+{


Per-tests configs aren't allowed. You may be able to use the test configs at https://github.com/ARMmbed/mbed-os/tree/master/tools/test_configs. @bridadan or @theotherjimmy may have more details around this.

Unfortunately that mechanism isn't really documented. It seems like you can add an entry to the "default_test_configuration" but I haven't used it personally.

What about --test-config <TEST_CONFIG> ?
It uses configs from https://github.com/ARMmbed/mbed-os/tree/master/tools/test_configs?

I believe so yes. If you don't specify a --test-config or an --app-config it seems to go with the default one.

c1728p9 · 2019-01-23T18:24:21Z

TESTS/usb_device/msd/main.cpp

+#define MIN_HEAP_SIZE (HEAP_BLOCK_DEVICE_SIZE + 6144)
+
+
+/* TODO:


Did you want to take action on this TODO?

Sure, as soon as it will be supported in USBMSD class

c1728p9 · 2019-01-23T18:28:35Z

TESTS/usb_device/msd/mbed_app.json

+            "target.features_add": ["STORAGE"]
+        },
+        "K64F": {
+            "target.components_remove": ["SD"]


Why does SD need to be removed from the K64F?

My assumption was to test embedded flash BD in the first place. In case of k64f it has components SD and FLASHAPI

mbed-os/targets/targets.json

Line 1443 in df9ac85

"components_add": ["SD", "FLASHIAP"],

Because the BlockDevice::get_default_instance() implementation uses SD before FLASHAPI
(see code below) we have to remove SD to let BlockDevice::get_default_instance() use FLASHAPI

mbed-os/features/storage/system_storage/SystemStorage.cpp

Lines 81 to 169 in df9ac85

MBED_WEAK BlockDevice *BlockDevice::get_default_instance()

{

#if COMPONENT_SPIF

static SPIFBlockDevice default_bd(

MBED_CONF_SPIF_DRIVER_SPI_MOSI,

MBED_CONF_SPIF_DRIVER_SPI_MISO,

MBED_CONF_SPIF_DRIVER_SPI_CLK,

MBED_CONF_SPIF_DRIVER_SPI_CS,

MBED_CONF_SPIF_DRIVER_SPI_FREQ

);

return &default_bd;

#elif COMPONENT_QSPIF

static QSPIFBlockDevice default_bd(

MBED_CONF_QSPIF_QSPI_IO0,

MBED_CONF_QSPIF_QSPI_IO1,

MBED_CONF_QSPIF_QSPI_IO2,

MBED_CONF_QSPIF_QSPI_IO3,

MBED_CONF_QSPIF_QSPI_SCK,

MBED_CONF_QSPIF_QSPI_CSN,

MBED_CONF_QSPIF_QSPI_POLARITY_MODE,

MBED_CONF_QSPIF_QSPI_FREQ

);

return &default_bd;

#elif COMPONENT_DATAFLASH

static DataFlashBlockDevice default_bd(

MBED_CONF_DATAFLASH_SPI_MOSI,

MBED_CONF_DATAFLASH_SPI_MISO,

MBED_CONF_DATAFLASH_SPI_CLK,

MBED_CONF_DATAFLASH_SPI_CS

);

return &default_bd;

#elif COMPONENT_SD

static SDBlockDevice default_bd(

MBED_CONF_SD_SPI_MOSI,

MBED_CONF_SD_SPI_MISO,

MBED_CONF_SD_SPI_CLK,

MBED_CONF_SD_SPI_CS

);

return &default_bd;

#elif COMPONENT_FLASHIAP

#if (MBED_CONF_FLASHIAP_BLOCK_DEVICE_SIZE == 0) && (MBED_CONF_FLASHIAP_BLOCK_DEVICE_BASE_ADDRESS == 0xFFFFFFFF)

size_t flash_size;

uint32_t start_address;

uint32_t bottom_address;

FlashIAP flash;

int ret = flash.init();

if (ret != 0) {

return 0;

}

//Find the start of first sector after text area

bottom_address = align_up(FLASHIAP_APP_ROM_END_ADDR, flash.get_sector_size(FLASHIAP_APP_ROM_END_ADDR));

start_address = flash.get_flash_start();

flash_size = flash.get_flash_size();

ret = flash.deinit();

static FlashIAPBlockDevice default_bd(bottom_address, start_address + flash_size - bottom_address);

#else

static FlashIAPBlockDevice default_bd;

#endif

return &default_bd;

#else

return NULL;

#endif

}

Perhaps it'd be possible to remove the dependency on the physical device all together? Could you use a HeapBlockDevice instead? https://github.com/ARMmbed/mbed-os/blob/master/features/storage/blockdevice/HeapBlockDevice.h

Not really. I'm already using heap block device.
The idea was to test more than one type of block device. The other problem is that many devices has not enough RAM to create 64kB heap block device (it's min size to mount FAT32 fs on it)

The idea was to test more than one type of block device

I get ya. Though I think the main thing you're trying to test is the USB stack's interaction with the FileSystem API, not the BlockDevice API, correct?

The other problem is that many devices has not enough RAM to create 64kB heap block device (it's min size to mount FAT32 fs on it)

Dang that's a pain. Crazy idea: maybe there's a way to simulate having a large block device? Especially if you don't actually have to store everything and just need to say the data moving. Almost like a streaming block device or something. (I may be going too far off topic now)

c1728p9 · 2019-01-23T18:29:20Z

targets/TARGET_NXP/TARGET_LPC176X/TARGET_ARCH_PRO/PinNames.h

+    SPI_MISO = P0_8,
+    SPI_SCK  = P0_7,
+    SPI_CS   = P0_6,
+


Can this change be upstreamed into master directly

Sure, I will do

Moved to master #9482

jupe · 2019-01-25T18:59:59Z

TESTS/host_tests/pyusb_msd.py

+
+        It will be used to find new disks mounted during test.
+        """
+        self.initial_disk_list = set(MSDUtils.disks())


how test would behave if we run multiple ones in same hosts parallel? test looks a bit problematic in ci where >>2 devices are connected in same host and devices appears and disappeared all the time.

In current version it will not work.
There is a plan to enhance host side msd device discovery/unmount mechanism similar as it's done in usb serial tests (https://github.com/ARMmbed/mbed-os/blob/feature-hal-spec-usb-device/TESTS/usb_device/serial distinguished by serial number)

@jupe serial number based host side storage detection was added 275d124
Serial number is generated so each test instance will have different one

Please re-review. Comments appear to be addressed

NirSonnenschein · 2019-02-06T13:24:12Z

@jupe @bridadan @c1728p9 please re-review latest changes.

bridadan · 2019-02-06T16:29:36Z

@NirSonnenschein I left some previous comments but I don't believe I need to approve this PR. Let me know if that's not the case, though.

cmonr · 2019-02-25T17:15:44Z

CI was started a bit ago.

mbed-ci · 2019-02-25T19:44:19Z

Test run: FAILED

Summary: 1 of 12 test jobs failed
Build number : 1
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_greentea-test

maciejbocianski · 2019-02-26T06:43:12Z

features-storage-tests-blockdevice-general_block_device failed:

[1551119791.78][CONN][INF] found KV pair in stream: {{__testcase_name;Testing get type functionality}}, queued... 
[1551119791.88][CONN][RXD] >>> Running case #1: 'Testing BlockDevice erase functionality'... [1551119791.88][CONN][RXD] [1551119791.88][CONN][INF] found KV pair in stream: {{__testcase_start;Testing BlockDevice erase functionality}}, queued... 
[1551119791.98][CONN][RXD] Test BlockDevice::get_erase_value().. 
[1551119841.99][CONN][RXD] :229::FAIL: Expected 0 Was -5005 
[1551119841.99][CONN][RXD] block_device->get_erase_value()=-1 
[1551119842.19][CONN][RXD] :234::SKIP: Erase not supported in this block device. Test skipped. [1551119842.19][CONN][INF] found KV pair in stream: {{__testcase_finish;Testing BlockDevice erase functionality;0;1}}, queued... 
[1551119842.29][CONN][RXD] >>> 'Testing BlockDevice erase functionality': 0 passed, 1 failed with reason 'Test Cases Failed'

cmonr · 2019-03-19T01:53:58Z

CI restarted

mbed-ci · 2019-03-19T06:27:38Z

Test run: SUCCESS

Summary: 13 of 13 test jobs passed
Build number : 2
Build artifacts

cmonr · 2019-03-19T20:19:22Z

One moment, doing a thing.

cmonr · 2019-03-19T20:20:52Z

continuous-integration/travis-ci/pr Pending — The Travis CI build is in progres

Now we're cooking! 😁

NirSonnenschein · 2019-03-20T09:39:05Z

Starting CI

mbed-ci · 2019-03-20T16:16:55Z

Test run: FAILED

Summary: 2 of 13 test jobs failed
Build number : 3
Build artifacts

Failed test jobs:

jenkins-ci/mbed-os-ci_cloud-client-test
jenkins-ci/mbed-os-ci_exporter

cmonr · 2019-03-20T18:25:49Z

CI job restarted: jenkins-ci/exporter

armclang: error: Failed to check out a license.

cmonr · 2019-03-20T18:27:33Z

CI job restarted: jenkins-ci/cloud-client-test

WebSocketProtocolException: rsv is not implemented, yet

maciejbocianski force-pushed the usb_msd_tests branch from 09e236e to 0e349e2 Compare January 21, 2019 14:59

ciarmcom requested review from c1728p9 and a team January 21, 2019 16:00

ciarmcom added the needs: review label Jan 21, 2019

maciejbocianski force-pushed the usb_msd_tests branch from 0e349e2 to 6087d8a Compare January 21, 2019 20:24

0xc0170 reviewed Jan 22, 2019

View reviewed changes

cmonr added needs: work and removed needs: review labels Jan 22, 2019

0xc0170 added needs: review and removed needs: work labels Jan 23, 2019

0xc0170 approved these changes Jan 23, 2019

View reviewed changes

c1728p9 previously requested changes Jan 23, 2019

View reviewed changes

maciejbocianski mentioned this pull request Jan 24, 2019

ARCH_PRO: add missing SPI pin definitions #9482

Merged

jupe reviewed Jan 25, 2019

View reviewed changes

0xc0170 added needs: work and removed needs: review labels Jan 28, 2019

c1728p9 force-pushed the feature-hal-spec-usb-device branch from 091b0e7 to e23e57d Compare January 29, 2019 18:42

cmonr added the needs: preceding PR label Jan 29, 2019

maciejbocianski force-pushed the usb_msd_tests branch 2 times, most recently from 26628b4 to 90c443a Compare January 30, 2019 08:55

cmonr added needs: review release-version: feature branch and removed needs: preceding PR needs: work labels Jan 30, 2019

0xc0170 removed the needs: review label Feb 25, 2019

cmonr added needs: work and removed needs: CI labels Feb 25, 2019

maciejbocianski added 2 commits February 27, 2019 17:18

USBMSD: Add media removal event check function

0cd9d24

Add USB mass storage test

7ff689b

maciejbocianski force-pushed the usb_msd_tests branch from 301f135 to 7ff689b Compare February 27, 2019 16:20

maciejbocianski changed the base branch from feature-hal-spec-usb-device to master February 27, 2019 16:23

cmonr added needs: CI release-version: 5.12.1 and removed needs: work release-version: feature branch labels Mar 19, 2019

cmonr closed this Mar 19, 2019

cmonr removed the needs: CI label Mar 19, 2019

cmonr reopened this Mar 19, 2019

cmonr added the needs: CI label Mar 19, 2019

0xc0170 added ready for merge and removed needs: CI labels Mar 21, 2019

0xc0170 merged commit f99431f into ARMmbed:master Mar 21, 2019

0xc0170 removed the ready for merge label Mar 21, 2019

		#define MIN_HEAP_SIZE (HEAP_BLOCK_DEVICE_SIZE + 6144)


		/* TODO:

	MBED_WEAK BlockDevice *BlockDevice::get_default_instance()
	{
	#if COMPONENT_SPIF

	static SPIFBlockDevice default_bd(
	MBED_CONF_SPIF_DRIVER_SPI_MOSI,
	MBED_CONF_SPIF_DRIVER_SPI_MISO,
	MBED_CONF_SPIF_DRIVER_SPI_CLK,
	MBED_CONF_SPIF_DRIVER_SPI_CS,
	MBED_CONF_SPIF_DRIVER_SPI_FREQ
	);

	return &default_bd;

	#elif COMPONENT_QSPIF

	static QSPIFBlockDevice default_bd(
	MBED_CONF_QSPIF_QSPI_IO0,
	MBED_CONF_QSPIF_QSPI_IO1,
	MBED_CONF_QSPIF_QSPI_IO2,
	MBED_CONF_QSPIF_QSPI_IO3,
	MBED_CONF_QSPIF_QSPI_SCK,
	MBED_CONF_QSPIF_QSPI_CSN,
	MBED_CONF_QSPIF_QSPI_POLARITY_MODE,
	MBED_CONF_QSPIF_QSPI_FREQ
	);

	return &default_bd;

	#elif COMPONENT_DATAFLASH

	static DataFlashBlockDevice default_bd(
	MBED_CONF_DATAFLASH_SPI_MOSI,
	MBED_CONF_DATAFLASH_SPI_MISO,
	MBED_CONF_DATAFLASH_SPI_CLK,
	MBED_CONF_DATAFLASH_SPI_CS
	);

	return &default_bd;

	#elif COMPONENT_SD

	static SDBlockDevice default_bd(
	MBED_CONF_SD_SPI_MOSI,
	MBED_CONF_SD_SPI_MISO,
	MBED_CONF_SD_SPI_CLK,
	MBED_CONF_SD_SPI_CS
	);

	return &default_bd;

	#elif COMPONENT_FLASHIAP

	#if (MBED_CONF_FLASHIAP_BLOCK_DEVICE_SIZE == 0) && (MBED_CONF_FLASHIAP_BLOCK_DEVICE_BASE_ADDRESS == 0xFFFFFFFF)

	size_t flash_size;
	uint32_t start_address;
	uint32_t bottom_address;
	FlashIAP flash;

	int ret = flash.init();
	if (ret != 0) {
	return 0;
	}

	//Find the start of first sector after text area
	bottom_address = align_up(FLASHIAP_APP_ROM_END_ADDR, flash.get_sector_size(FLASHIAP_APP_ROM_END_ADDR));
	start_address = flash.get_flash_start();
	flash_size = flash.get_flash_size();

	ret = flash.deinit();

	static FlashIAPBlockDevice default_bd(bottom_address, start_address + flash_size - bottom_address);

	#else

	static FlashIAPBlockDevice default_bd;

	#endif

	return &default_bd;

	#else

	return NULL;

	#endif

	}

Usb msd tests #9444

Usb msd tests #9444

Uh oh!

Conversation

maciejbocianski commented Jan 21, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

TODO:

Known issues:

Pull request type

Reviewers

Uh oh!

0xc0170 commented Jan 21, 2019

Uh oh!

ciarmcom commented Jan 21, 2019

Uh oh!

maciejbocianski commented Jan 21, 2019

Uh oh!

0xc0170 Jan 22, 2019 • 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!

Choose a reason for hiding this comment

Uh oh!

c1728p9 left a comment • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

maciejbocianski Jan 25, 2019 • 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!

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!

maciejbocianski Jan 24, 2019 • 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!

Choose a reason for hiding this comment

Uh oh!

maciejbocianski Jan 25, 2019 • 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!

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!

Choose a reason for hiding this comment

Uh oh!

maciejbocianski Jan 28, 2019 • 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!

maciejbocianski Feb 5, 2019 • 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!

NirSonnenschein commented Feb 6, 2019

Uh oh!

bridadan commented Feb 6, 2019

Uh oh!

cmonr commented Feb 25, 2019

Uh oh!

mbed-ci commented Feb 25, 2019

Uh oh!

maciejbocianski commented Jan 21, 2019 •

edited

Loading

0xc0170 Jan 22, 2019 •

edited

Loading

c1728p9 left a comment •

edited

Loading

maciejbocianski Jan 25, 2019 •

edited

Loading

maciejbocianski Jan 24, 2019 •

edited

Loading

maciejbocianski Jan 25, 2019 •

edited

Loading

maciejbocianski Jan 28, 2019 •

edited

Loading

maciejbocianski Feb 5, 2019 •

edited

Loading