Update documentation for target definitions #398

drewcassidy · 2018-02-06T06:16:41Z

Adds some much-needed documentation on how to use the config key when
defining a new target, as well as overriding it in other targets or in
the mbed_app.json file

This doesnt show in the rendered HTML, but helps when editing the file and when viewing it through github

goes along with ARMmbed/mbed-os#5910

Adds some much-needed documentation on how to use the `config` key when defining a new target, as well as overriding it in other targets or in the mbed_app.json file

Copy edit for active voice, consistent tense and formatting.

AnotherButler

@drewcassidy Thanks for the update. I've left a few questions for you to address.

AnotherButler · 2018-02-06T19:34:24Z

docs/tools/mbed_targets.md

@@ -148,6 +149,40 @@ The build system errors when you use features outside of this list.

 When you use target inheritance, you may alter the values of `features` using `features_add` and `features_remove`. This is similar to the `macros_add` and `macros_remove` mechanism the previous section describes.

+#### `config` and `overrides`
+
+_configs_ provide a more flexible way to manage macros for a target. Each configuration has a macro name, as well as a default value and an optional help value. For example, this config flag defines a configuration `lf_clock_src` that defines the source for the low frequency clock on the nRF51 target, with a default value of `NRF_LF_SRC_XTAL`. Compiling sets this value for the macro `MBED_CONF_NORDIC_NRF_LF_CLOCK_SRC`:


Query: In "a more flexible way", what are we comparing this to? More flexible than what?

Also, could you please clarify the third sentence? When we say "this config flag," are we talking about the example starting on line 156 or something else?

Also, is this example specific to the nRF51, or does it work with other boards, too?

I was referencing the section of the style guide below on why modules and boards shouldnt use macros. The example is just a config definition I pulled from the NRF51 target, but I'll replace it with something more generic. I should probably move the explanation after the example

AnotherButler · 2018-02-06T19:34:47Z

docs/tools/mbed_targets.md

+}
+```
+
+_overrides_ allow a child target to change the value of a config. For example, if a child target of the nRF51 uses the internal RC clock instead of the crystal, it can add an override:


Query: Is this example specific to the nRF51, or does it work for other boards, too?

update description of the `config` flag to remove anything specific to the nRF51, as well as moving explanations after example snippets

0xc0170 · 2018-02-07T08:28:41Z

docs/tools/mbed_targets.md

@@ -290,9 +330,9 @@ For each of these target roles, some restrictions are in place:
  - `extra_labels`.
  - `public`.
  - `config`.
+  - `override`


isn't this overrides (s missing in here) ?

0xc0170 · 2018-02-07T08:29:56Z

docs/tools/mbed_targets.md

+}
+```
+
+ This case defines the config `clock_src`, for the `CLOCK_SRC` macro, and has a default value of `XTAL`.


wouldn't be easier to show here default naming convention for config and also mention that macro_name overrides it ?

ah, I didnt even realize it defaults to the config name.

"override" -> "overrides"

clarify that configs default to their name for the macro they apply to, and use the same example configs throughout the section

0xc0170 · 2018-02-08T12:44:30Z

docs/tools/mbed_targets.md

    }
 }
 ```

- This case defines the config `clock_src`, for the `CLOCK_SRC` macro, and has a default value of `XTAL`.
+ This case defines the config `clock_src` with the default value of `XTAL` for the macro `CLOCK_SRC`, and the config `clock_freq` with the default value of 16 for the macro `CLOCK_FREQUENCY_MHZ`.


have you tested it ? Ibelieve the default name for clock_src value will be MBED_CONF_CLOCK_SRC ?

I didnt realize there was a standardized macro for that

would something like the bluetooth advertising name be a better example maybe?

drewcassidy · 2018-02-08T20:32:39Z

So while trying to find a better config example I found this page from the older docs, should we just reuse the same thing? it looks like its still up to date

or, actually there is this page which should probably just be expanded and linked from the targets page

AnotherButler · 2018-02-08T20:50:02Z

We do have this page about the configuration system: https://os.mbed.com/docs/v5.7/tools/configuring-tools.html

Would it make sense to link to it from the mbed_targets page?

drewcassidy · 2018-02-08T20:53:10Z

ah, yeah I just found that. That would probably make sense with just a brief description of the system in mbed_targets

it's a bit hard to find these documentation pages through google because of all the older versions showing up. I'm wonder if there should be a change to the docs website's robots.txt

Add link to config page and copy edit file for active voice and consistent tense across docs.

AnotherButler · 2018-03-05T22:47:34Z

@drewcassidy I've added the link to the config page. I also changed this to point to our test site (the new_engine branch), so we can test it before we make it live. Could you please rebase this to resolve the merge conflicts?

Make changes from PR #398 to new_engine, so they go to the test site for review.

AnotherButler · 2018-03-09T22:44:05Z

Closing because I'm copying these changes against new_engine

drewcassidy added 3 commits January 23, 2018 16:02

add language hilighting to mbed_targets.md

0db3f75

This doesnt show in the rendered HTML, but helps when editing the file and when viewing it through github

fix typo in example output

8a89a5b

goes along with ARMmbed/mbed-os#5910

Add documentation on configs

cfebd4b

Adds some much-needed documentation on how to use the `config` key when defining a new target, as well as overriding it in other targets or in the mbed_app.json file

AnotherButler requested a review from 0xc0170 February 6, 2018 19:27

Copy edit mbed_targets.md

b20088f

Copy edit for active voice, consistent tense and formatting.

AnotherButler reviewed Feb 6, 2018

View reviewed changes

drewcassidy added 2 commits February 6, 2018 22:55

Typo fix

1c60daf

Update config description

f695f4f

update description of the `config` flag to remove anything specific to the nRF51, as well as moving explanations after example snippets

drewcassidy force-pushed the 5.7 branch from 1e4c677 to f695f4f Compare February 7, 2018 07:02

0xc0170 reviewed Feb 7, 2018

View reviewed changes

drewcassidy mentioned this pull request Feb 7, 2018

add OSHChip as an mbed target ARMmbed/mbed-os#5892

Merged

3 tasks

drewcassidy added 4 commits February 7, 2018 10:59

Fix typo

5073446

"override" -> "overrides"

add period like other items in this list

9a67ee3

clarify use of config name as macro

86ddcea

clarify that configs default to their name for the macro they apply to, and use the same example configs throughout the section

minor grammatical fixes

b4ccba0

0xc0170 reviewed Feb 8, 2018

View reviewed changes

Edit mbed_targets.md

b280333

Add link to config page and copy edit file for active voice and consistent tense across docs.

AnotherButler changed the base branch from 5.7 to new_engine March 5, 2018 22:46

AnotherButler changed the base branch from new_engine to 5.7 March 9, 2018 22:40

AnotherButler pushed a commit that referenced this pull request Mar 9, 2018

Update mbed_targets.md

a084a49

Make changes from PR #398 to new_engine, so they go to the test site for review.

AnotherButler closed this Mar 9, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Update documentation for target definitions #398

Update documentation for target definitions #398

Uh oh!

drewcassidy commented Feb 6, 2018

Uh oh!

AnotherButler left a comment

Uh oh!

AnotherButler Feb 6, 2018

Uh oh!

drewcassidy Feb 6, 2018

Uh oh!

AnotherButler Feb 6, 2018

Uh oh!

0xc0170 Feb 7, 2018

Uh oh!

0xc0170 Feb 7, 2018

Uh oh!

drewcassidy Feb 7, 2018

Uh oh!

0xc0170 Feb 8, 2018

Uh oh!

drewcassidy Feb 8, 2018

Uh oh!

drewcassidy Feb 8, 2018

Uh oh!

drewcassidy commented Feb 8, 2018 •

edited

Loading

Uh oh!

AnotherButler commented Feb 8, 2018

Uh oh!

drewcassidy commented Feb 8, 2018

Uh oh!

AnotherButler commented Mar 5, 2018

Uh oh!

AnotherButler commented Mar 9, 2018

Uh oh!

Uh oh!

Update documentation for target definitions #398

Update documentation for target definitions #398

Uh oh!

Conversation

drewcassidy commented Feb 6, 2018

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!

Choose a reason for hiding this comment

Uh oh!

drewcassidy commented Feb 8, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

AnotherButler commented Feb 8, 2018

Uh oh!

drewcassidy commented Feb 8, 2018

Uh oh!

AnotherButler commented Mar 5, 2018

Uh oh!

AnotherButler commented Mar 9, 2018

Uh oh!

Uh oh!

drewcassidy commented Feb 8, 2018 •

edited

Loading