Add default block device support (SD, SPIF and FLASHIAP) documentation #685

yossi2le · 2018-08-26T13:48:32Z

This PR is intended to update the documentation of block devices and filesystems after the merge of PR ARMmbed/mbed-os#7774 into mbed-os.

The documentation contains

Change of paths.
Update new implementation of get default instance for BlockDevice and FileSystem.
Importing into mbed-os and modify the documentation of external SD, SPIF, DATAFLASH and FLASHIAP drivers

Please refer to PR #7774 in the link above for all details regarding this change.

…vice FileSystem documentation for new get_default_instance implementation

AnotherButler · 2018-08-28T21:25:04Z

docs/reference/api/storage/BlockDevice.md

@@ -30,6 +30,66 @@ The state of an erased block is **undefined**. The data stored on the block isn'

 ![blockdevicesectors](https://s3-us-west-2.amazonaws.com/mbed-os-docs-images/blockdevice_erase_block.png)

+### BlockDevice get default instance


Can we add this to the storage configuration page?

https://github.com/ARMmbed/mbed-os-5-docs/blob/development/docs/reference/configuration/Storage.md

Maybe add a BlockDevice section?

Copy edit file, mostly for grammar and style consistency.

Make initial copy edits, mostly for formatting.

Fix typo.

Copy edit for spelling and minor style nits.

Remove extra information from user API, and copy edit remaining text.

AnotherButler

Please address my comments.

AnotherButler · 2018-08-28T21:30:23Z

docs/reference/api/storage/FileSystem.md

@@ -6,6 +6,34 @@ The main purpose of a FileSystem is to be instantiated. The FileSystem's constru

 The FileSystem's `file` and `dir` functions are protected because you should not use the FileSystem `file` and `dir` functions directly. They are only a convenience for implementors. Instead, the [File](file.html) and [Dir](dir.html) classes provide access to file and directory operations in a C++ class that respects [RAII](/docs/development/introduction/glossary.html#r) and other C++ conventions.

+### File system get default instance


Query: Can we move the configuration content to the storage config page?

AnotherButler · 2018-08-28T21:35:59Z

docs/reference/api/storage/FlashIAPBlockDevice.md

+
+## FlashIAPBlockDevice class reference
+
+[![View code](https://www.mbed.com/embed/?type=library)](<Should be added after doxygen run>)


Query: Do we have an example for this yet? If not, can you create one?

no, but I will add a code snippet to help people understand how to use it.

AnotherButler · 2018-08-28T21:36:18Z

docs/reference/api/storage/FlashIAPBlockDevice.md

@@ -0,0 +1,17 @@
+## FlashIAPBlockDevice
+
+The flash IAP block device is a block device driver built on top of the FlashIAP API.


Please add more description of what this API does and its use cases.

AnotherButler · 2018-08-28T21:36:54Z

docs/reference/api/storage/FlashIAPBlockDevice.md

+
+The flash IAP block device is a block device driver built on top of the FlashIAP API.
+
+## Warning


We need to have Product review this warning.

This warning should be removed.

AnotherButler · 2018-08-28T22:01:26Z

docs/reference/api/storage/SDBlockDevice.md

+- The file system code (hosted in `mbed-os`) is composed of 2 parts: the FAT file system implementation code and the file system classes that present a consistent API to the retarget module for different (third-party) file system implementations.
+- The Block API. The SDCard block device is a persistent storage block device.
+- The SPI module provides the Mbed OS SPI API.
+


Please add a section here linking to our Doxygen.

ashok-rao · 2018-08-29T15:54:59Z

docs/reference/api/storage/BlockDevice.md

+Please note that while a default block device exists an application is not enforced to use it and can create its own one.
+
+### Overriding default block device implementation 
+The get default instance is implemented as MBED_WEAK at features/storage/system_storage/SystemStorage.cpp. That means that it can be overridden by implementing the function without MBED_WEAK and change the default block device for a given application.


Maybe a link to -> this will be useful ?? it's not immediately clear to me what the macro means..

yossi2le · 2018-08-30T09:04:59Z

Hi @AnotherButler. I will update the PR today with changes addressing all comments.
@ashok-rao I am moving the configuration and overriding to storage configuration guide. I will add the link you ask over there.
Thank you all, stay tuned.

….md config. Adding info regarding FlashIAPBlockDevice.

offirko

No comments, looks great.

Copy edit file, mostly for consistent tense across docs.

Copy edit file, mostly for active voice.

Delete duplicate content.

Copy edit file for clarity.

Copy edit file for active voice, present tense and other grammar.

AnotherButler · 2018-08-30T21:52:17Z

docs/reference/configuration/Storage.md

+
+#### Configuring components
+
+Adding "components": ["???"] in targets.json:


@theotherjimmy As it's the weekend in Israel, could you please tell me what should go here?

I think that's going to be one of "SD", "SPIF" or "FLASHIAP"

This should just say "for example, the following entry in targets.json enables the SD component". The following example should be trimmed to only identifying info and the important information.

"K64F": { "components": ["SD"], "core": "Cortex-M4F", "supported_toolchains": ["ARM", "GCC_ARM", "IAR"], "inherits": ["Target"], "features": ["STORAGE"], "release_versions": ["2", "5"], ... },

Is probably enough to get the point across.

AnotherButler · 2018-08-30T21:52:23Z

docs/reference/configuration/Storage.md

+    },
+```
+
+Adding "target.components_add": ["???"] in application config file:


@theotherjimmy As it's the weekend in Israel, could you please tell me what should go here?

Same as above.

So could I change it to

Adding "target.components_add": [""] in application config file:

For clarity?

No. That particular line is somewhat nonsense, somewhat leads to unexpected behavior.

I recommend:
"The following mbed_app.json snippet enables the Spi flash component when compiling for the MTB_ADV_WISE_1570 target.

"MTB_ADV_WISE_1570": { "target.components_add": ["SPIF"] }

(removed unrelated lines from the example)

@theotherjimmy Let's keep the comments professional and kind. I know @yossi2le and I both appreciate your help streamlining the example, but it doesn't mean the redundant lines are "garbage".

As in "things to be thrown out"? I could have picked a more constructive word for sure. That word may have been in poor taste. Edited.

Personally I think unrelated lines take away from an example as they have the potential to distract or misdirect the reader and I find that having them is a determent to the example.

I meat no ill will or disparaging remarks to anyone or their writing skills/style.

Honestly, The beginning of that comment was really worse so I edited that out too.

@theotherjimmy No hard feelings;)

AnotherButler · 2018-08-30T21:52:48Z

docs/reference/api/storage/DataFlashBlockDevice.md

+
+### DataFlashBlockDevice class reference
+
+[![View code](https://www.mbed.com/embed/?type=library)](<Should be added after doxygen run>)


Note to self: Add link after code merges.

AnotherButler · 2018-08-30T21:53:08Z

docs/reference/api/storage/FlashIAPBlockDevice.md

+
+## FlashIAPBlockDevice class reference
+
+[![View code](https://www.mbed.com/embed/?type=library)](<Should be added after doxygen run>)


Note to self: Add link after code merges.

AnotherButler · 2018-08-30T21:53:24Z

docs/reference/api/storage/SDBlockDevice.md

+
+### SDBlockDevice class reference
+
+[![View code](https://www.mbed.com/embed/?type=library)](<Should be added after doxygen run>)


Note to self: Add link after code merges.

AnotherButler · 2018-08-30T21:53:36Z

docs/reference/api/storage/SPIFBlockDevice.md

+
+### SPIFBlockDevice class reference
+
+[![View code](https://www.mbed.com/embed/?type=library)](<Should be added after doxygen run>)


Note to self: Add link after code merges.

Update snippets with recommended changes for clarity.

yossi2le · 2018-09-02T11:42:54Z

@AnotherButler Thanks for the changes.

AnotherButler · 2018-09-04T11:18:59Z

@yossi2le I can't find the code in the Doxygen: https://os-doc-builder.test.mbed.com/docs/development/mbed-os-api-doxy/annotated.html

Could you please help me?

yossi2le · 2018-09-04T11:33:48Z

@yossi2le I can't find the code in the Doxygen: https://os-doc->builder.test.mbed.com/docs/development/mbed-os-api-doxy/annotated.html

Could you please help me?

I wish I could. I am puzzled, are we looking at the correct build.
its written at the end of the BlockDevice documentation:

The documentation for this class was generated from the following file:
mbed-os/features/filesystem/bd/BlockDevice.h

However, this file has been moved to mbed-os/features/storage/blockdevice/BlockDevice.h

I have a feeling this Doxygen output is before #7774

AnotherButler · 2018-09-04T11:35:52Z

@kegilbert Is this something you can help with?

AnotherButler · 2018-09-04T11:48:09Z

@yossi2le @kegilbert I'm going to comment out the broken links until we can fix them. That way, I can merge this, so OOB can start reviewing the descriptions and examples. Please do not stop working to figure out why these links don't work just because this PR is closing.

Comment out missing link.

Temporarily comment out missing link.

yossi2le · 2018-09-04T11:51:41Z

@AnotherButler
Like I wrote before I am guessing the links are broken because the Doxygen generated not from the most updated master.

Comment out missing link.

AnotherButler · 2018-09-11T21:52:00Z

@yossi2le @kegilbert I've updated the Doxygen, and these classes still aren't appearing: https://os-doc-builder.test.mbed.com/docs/development/mbed-os-api-doxy/annotated.html

Should they be under Data Structures, where I'm looking? Or are they somewhere else?

Fix broken link now that Doxygen's fixed.

Add link now that Doxygen's fixed.

Adding new block devices in mbedos to the handbook and update BlockDe…

ef4e764

…vice FileSystem documentation for new get_default_instance implementation

AnotherButler added the code dependency label Aug 27, 2018

AnotherButler requested review from MarceloSalazar, ashok-rao, offirko and AnotherButler August 28, 2018 21:17

AnotherButler reviewed Aug 28, 2018

View reviewed changes

Amanda Butler added 5 commits August 28, 2018 16:29

Copy edit DataFlashBlockDevice.md

bcf07fa

Copy edit file, mostly for grammar and style consistency.

Make initial copy edits on FlashIAPBlockDevice.md

6d00981

Make initial copy edits, mostly for formatting.

Fix typo in BlockDevice.md

b2044c6

Fix typo.

Copy edit SPIFBlockDevice.md

8c64c57

Copy edit for spelling and minor style nits.

Edit SDBlockDevice to match our template

575babf

Remove extra information from user API, and copy edit remaining text.

AnotherButler suggested changes Aug 28, 2018

View reviewed changes

ashok-rao reviewed Aug 29, 2018

View reviewed changes

Moving default block device and filesystem configuration into storage…

dfe7e4a

….md config. Adding info regarding FlashIAPBlockDevice.

offirko approved these changes Aug 30, 2018

View reviewed changes

Amanda Butler added 6 commits August 30, 2018 12:33

Copy edit BlockDevice.md

1707af5

Copy edit file, mostly for consistent tense across docs.

Copy edit FileSystem.md

656e608

Copy edit file, mostly for active voice.

Delete content in BlockDevice.md

b3587da

Delete duplicate content.

Delete content in FileSystem.md

e5abaf2

Delete duplicate content.

Copy edit FlashIAPBlockDevice.md

fe51f3d

Copy edit file for clarity.

Copy edit Storage.md

b49f209

Copy edit file for active voice, present tense and other grammar.

AnotherButler reviewed Aug 30, 2018

View reviewed changes

Update snippets in Storage.md

164c5aa

Update snippets with recommended changes for clarity.

AnotherButler added 5.10 urgent and removed code dependency labels Sep 4, 2018

Amanda Butler added 2 commits September 4, 2018 06:50

Remove missing link from DataFlashBlockDevice.md

d751087

Comment out missing link.

Remove link from FlashIAPBlockDevice.md

6aa1439

Temporarily comment out missing link.

Amanda Butler added 2 commits September 4, 2018 06:52

Remove link from SDBlockDevice.md

ec40f08

Comment out missing link.

Remove link to SPIFBlockDevice.md

bd34418

Comment out missing link.

kegilbert mentioned this pull request Sep 12, 2018

Allow storage doxy to build ARMmbed/mbed-os#8100

Merged

AnotherButler added the code dependency label Sep 13, 2018

Amanda Butler added 4 commits September 17, 2018 15:30

Fix link in DataFlashBlockDevice.md

a99a390

Fix broken link now that Doxygen's fixed.

Add link to FlashIAPBlockDevice.md

9ae0dbf

Add link now that Doxygen's fixed.

Add link to SDBlockDevice.md

f05252d

Add link now that Doxygen's fixed.

Add link to SPIFBlockDevice.md

3759d01

Add link now that Doxygen's fixed.

AnotherButler approved these changes Sep 17, 2018

View reviewed changes

AnotherButler removed the code dependency label Sep 17, 2018

AnotherButler merged commit 1becbc4 into ARMmbed:development Sep 17, 2018

		@@ -30,6 +30,66 @@ The state of an erased block is undefined. The data stored on the block isn'

		![blockdevicesectors](https://s3-us-west-2.amazonaws.com/mbed-os-docs-images/blockdevice_erase_block.png)

		### BlockDevice get default instance

		@@ -6,6 +6,34 @@ The main purpose of a FileSystem is to be instantiated. The FileSystem's constru

		The FileSystem's `file` and `dir` functions are protected because you should not use the FileSystem `file` and `dir` functions directly. They are only a convenience for implementors. Instead, the [File](file.html) and [Dir](dir.html) classes provide access to file and directory operations in a C++ class that respects [RAII](/docs/development/introduction/glossary.html#r) and other C++ conventions.

		### File system get default instance


		## FlashIAPBlockDevice class reference

		[![View code](https://www.mbed.com/embed/?type=library)](<Should be added after doxygen run>)

		@@ -0,0 +1,17 @@
		## FlashIAPBlockDevice

		The flash IAP block device is a block device driver built on top of the FlashIAP API.


		The flash IAP block device is a block device driver built on top of the FlashIAP API.

		## Warning


		#### Configuring components

		Adding "components": ["???"] in targets.json:


		### DataFlashBlockDevice class reference

		[![View code](https://www.mbed.com/embed/?type=library)](<Should be added after doxygen run>)


		### SDBlockDevice class reference

		[![View code](https://www.mbed.com/embed/?type=library)](<Should be added after doxygen run>)


		### SPIFBlockDevice class reference

		[![View code](https://www.mbed.com/embed/?type=library)](<Should be added after doxygen run>)

Add default block device support (SD, SPIF and FLASHIAP) documentation #685

Add default block device support (SD, SPIF and FLASHIAP) documentation #685

Uh oh!

Conversation

yossi2le commented Aug 26, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AnotherButler left a comment

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!

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!

yossi2le commented Aug 30, 2018

Uh oh!

offirko left a comment

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!

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!

theotherjimmy Aug 31, 2018 • 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!

theotherjimmy Sep 4, 2018 • 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!

yossi2le commented Sep 2, 2018

Uh oh!

AnotherButler commented Sep 4, 2018

Uh oh!

yossi2le commented Sep 4, 2018

Uh oh!

AnotherButler commented Sep 4, 2018

Uh oh!

AnotherButler commented Sep 4, 2018

Uh oh!

yossi2le commented Sep 4, 2018

Uh oh!

theotherjimmy Aug 31, 2018 •

edited

Loading

theotherjimmy Sep 4, 2018 •

edited

Loading