doc/rdm: RDM on a common api for testing by MrKevinWeiss · Pull Request #10624 · RIOT-OS/RIOT

MrKevinWeiss · 2018-12-18T10:29:20Z

Contribution description

This is the initial commit of Guidelines for Write Firmware to Expose RIOT APIs RDM.
It attempts to standardize some semantics for firmware written in tests. This should allow easier parsing, sorting and filtering of useful information for tests and experiments. without negatively effecting other parts of RIOT. It's aim is to provide answers to simple questions of something like, what should I expect at the end of a test, "success", "Success", "SUCCESS", "PASS", "FAIL", "ERROR", "-1", "0". Something we can all agree upon and, if we adhere to it utilize the standardization for other things.

This is the initial commit of Guidelines for Write Firmware to Expose RIOT APIs RDM. It attempts to standardize some semantics for firmware written in tests.

jcarrano

Some comments regarding the output format.

jcarrano · 2018-12-19T10:24:48Z

doc/memos/rdm-draft-weiss-exposing-api-firmware-guidelines.md

+information
+- [data] should be optional
+- [data] if used, may be provided several times
+- [data] should be actionable and adhere to pythons


Why not use something compatible with JSON?

a. There is code to generate that already
b. You don't tie the receiving end to python.

I like the idea, if I use standard json do I need to know the full frame? I would worry more about the coordination the messages need.

Here I'm assuming you are going to put a single [data] element per line. If that is not the case, it is not a problem either, because json can be parsed from a stream (meaning you could pretty format the [data] into multiple lines.)

Ok, can we say a either json compatible line or generic string which defaults to the [msg] form? I will update the examples.

jcarrano · 2018-12-19T10:25:13Z

doc/memos/rdm-draft-weiss-exposing-api-firmware-guidelines.md

+- [msg] should be optional
+- [msg] if used, may be provided several times
+- [msg] should not be actionable and only contain debug/log
+information


I think that "[msg] should not start with {" and data should. That way it is unambiguous.

The reason for this is a mechanism to catch anything that shouldn't be formatted. This way not data can be missed.

Ok, but how do you differentiate between data and message. Another option is to say that anything that cannot be successfully parsed as data is a message.

Agreed that's what I will do!

jcarrano · 2018-12-19T10:26:41Z

doc/memos/rdm-draft-weiss-exposing-api-firmware-guidelines.md

+- [data] if used, may be provided several times
+- [data] should be actionable and adhere to pythons
+ast.literal_eval formatting
+- [result] should only be provided once for each command


"At most once" or "exacly once"?

Exactly, I will update! It is used to end the message so we don't have to wait for timeouts.

jcarrano · 2018-12-19T10:34:08Z

doc/memos/rdm-draft-weiss-exposing-api-firmware-guidelines.md

+- [cmd] should only be provided once for each command
+- [cmd] should allow the user to reproduce the step, this may be the
+API/function call with the variables or a descriptive name for a set of calls
+- [msg] should be optional


Instead of writing so much text, you could roughly specify the grammar of the response:

response := message* data* result message := MESSAGE_CONTENTS EOL data := <json> EOL result := "{" "'result'" ":" RESULT_VALUE "}" EOL EOL := '\n' MESSAGE_CONTENTS := ([^{].+)? RESULT_VALUE := (SUCCESS) | (ERROR)

Can I have both? I will add that in.

MrKevinWeiss · 2018-12-19T10:43:42Z

Thanks @jcarrano, that is a very useful review!

MichelRottleuthner

Thanks a lot for addressing this! I don't have much to say for now because IMO agreeing on any structured format would already be an improvement. Minor things I noticed below.

MichelRottleuthner · 2019-01-29T15:53:01Z

doc/memos/rdm-draft-weiss-exposing-api-firmware-guidelines.md

+- Tests should have a `get_metadata` command
+- The `get_metadata` command should contain at least board name, and
+application name
+- Tests should have a `echo <random 8 hex digits>` command


The commands get_metadata and echo should probably just be implemented once and then be included in all tests that provide the interface specified by this RDM

Yup, I will try to implement something, I suppose it would be like the ps/reboot/ect. where you can just include it so there will be no code duplication.

MichelRottleuthner · 2019-01-29T16:30:15Z

doc/memos/rdm-draft-weiss-exposing-api-firmware-guidelines.md

+## Abstract
+
+In order to improve the testing and interface tools for RIOT a standard way of
+exposing API calls to a higher level should be defined. This document describes


exposing API calls to a higher level

Can this be rephrased to directly state that this is talking about a shell interface that gives access to a RIOT firmware? A higher level is somewhat ambiguous in this sentence and IMO it is worth to mention that this is talking about tools that are running outside of an RIOT instance.

YES: It will be really helpful to make the context and subject of consideration crisply clear so that the reader understands right away what to expect from this memo.

Ditto, NB that "exposing API calls" generally refers to a module exposing functions for use outside itself in a software program - it wouldn't really imply "allowing a human to use those functions through a user interface"

OK, I will try to rephrase it in a clearer way.

danpetry · 2019-01-31T14:58:22Z

My initial thoughts are that the way these rules are specified would make applying them quite onerous for developers: they would have to go through all bullet points one by one. Perhaps a format such as a commented template (or maybe condensing and structuring them somewhat) would make it easier for people to use (and more likely for it to get used therefore).

danpetry · 2019-01-31T15:03:51Z

Also, is there any scope for this document to take into account other aspects of testing? For example, when things should be unit tested using embunit or have CLI tests written using testrunner, and the kind of test coverage that's expected? Or outlining what our eventual/ideal CI strategy would be? The testing in general is very varied seemingly without strong common logic in what is tested and how, not just in the area of how tests are exposed through CLI output

MrKevinWeiss · 2019-02-01T16:13:17Z

Perhaps a format such as a commented template (or maybe condensing and structuring them somewhat)

Can you show me what you mean, maybe give an example?

MrKevinWeiss · 2019-02-01T16:28:46Z

is there any scope for this document to take into account other aspects of testing?

The idea is it provides some standard to write to, it should only provide answers to the question of, for example, how should I format my data. It is not meant to cover all test cases only provide a standard for those tests that fit the criteria. Embunit seems like it is more for c unit tests not really requiring this and test runner could use this since it should make the parsing easier.

Or outlining what our eventual/ideal CI strategy would be?

That is too large a scope for the document (that was how it started but was suggested to reduce the scope). This is useful regardless of the CI strategy but yes, it is intended to help with the CI development.

The testing in general is very varied seemingly without strong common logic in what is tested and
how, not just in the area of how tests are exposed through CLI output.

Yup, I want to only bite off what I can chew...

It will be easier to get a consensus if we keep things simple and obviously beneficial. Other standardization/guidelines/templates can also come later.

danpetry · 2019-02-04T10:53:06Z

Perhaps a format such as a commented template (or maybe condensing and structuring them somewhat)

Can you show me what you mean, maybe give an example?

Maybe presenting the details of [msg data result] etc would look better in a table. On second look the spec is probably too generic to provide a single template although the code examples help. In general some other way of presenting than a long list of bullet points would be good. Potentially breaking up the points into subsections, or enumerating them, or using tables might help break it up/give it some high level structure to make the points more easy to understand/digest/contextualise

danpetry · 2019-02-04T10:53:56Z

that was how it started but was suggested to reduce the scope

ok, understood

MrKevinWeiss · 2019-02-06T09:55:56Z

doc/memos/rdm-draft-weiss-exposing-api-firmware-guidelines.md

+     printf("Getting several values for the api command\n")
+     for (i = 0; i < amount_of_api_calls; i++) {
+         data = get_data_api_command(i);
+         prqintf("{\"data\": 0x%X}\n", data);


stale · 2019-08-10T10:07:51Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.

MrKevinWeiss · 2019-08-12T13:01:02Z

I am currently working on implementation of this. It will require an update but I don't think it will be forgotten!

stale · 2020-02-13T13:07:28Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. If you want me to ignore this issue, please mark it with the "State: don't stale" label. Thank you for your contributions.

MrKevinWeiss · 2021-12-02T08:28:36Z

I think this is out of date and somehow a part of the turo + riotctrl.

MrKevinWeiss added Area: doc Area: Documentation Discussion: RFC The issue/PR is used as a discussion starting point about the item of the issue/PR Process: needs >1 ACK Integration Process: This PR requires more than one ACK Area: RDM Area: Discussions on RIOT Developer Memos labels Dec 18, 2018

MrKevinWeiss self-assigned this Dec 18, 2018

MrKevinWeiss requested review from emmanuelsearch, haukepetersen, smlng, tcschmidt and waehlisch December 18, 2018 10:29

MrKevinWeiss force-pushed the pr/doc/exposingapirdm branch from e3732fd to 477d0c1 Compare December 18, 2018 10:31

doc/rdm: Add exposing api rdm

f03a630

This is the initial commit of Guidelines for Write Firmware to Expose RIOT APIs RDM. It attempts to standardize some semantics for firmware written in tests.

MrKevinWeiss force-pushed the pr/doc/exposingapirdm branch from 477d0c1 to f03a630 Compare December 18, 2018 10:32

jcarrano reviewed Dec 19, 2018

View reviewed changes

fixup: formatted output for JSON compatability

049ab66

smlng mentioned this pull request Jan 22, 2019

tests: init RobotFramework and add basic xtimer test #10843

Closed

MrKevinWeiss mentioned this pull request Jan 25, 2019

RFC: UART enhanced settings #10743

Merged

waehlisch changed the title ~~doc/rdm: Add exposing api rdm~~ doc/rdm: RDM on exposing an api for testing Jan 28, 2019

waehlisch changed the title ~~doc/rdm: RDM on exposing an api for testing~~ doc/rdm: RDM on common exposing an api for testing Jan 28, 2019

waehlisch changed the title ~~doc/rdm: RDM on common exposing an api for testing~~ doc/rdm: RDM on a common api for testing Jan 28, 2019

waehlisch requested a review from cladmi January 29, 2019 16:46

MichelRottleuthner reviewed Jan 29, 2019

View reviewed changes

MrKevinWeiss commented Feb 6, 2019

View reviewed changes

MrKevinWeiss mentioned this pull request Feb 6, 2019

sys/app_metadata: Add a way to print application metadata #10961

Merged

jcarrano mentioned this pull request Feb 11, 2019

[TRACKING] The RIOT shell as a testing interface #10994

Open

9 tasks

jcarrano added a commit to jcarrano/RIOT that referenced this pull request Mar 13, 2019

tests/malloc: adapt command line to the standard in RIOT-OS#10624

648b01b

jcarrano mentioned this pull request Apr 15, 2019

sys/optparse: Add command line parser. #9538

Closed

stale bot added the State: stale State: The issue / PR has no activity for >185 days label Aug 10, 2019

MrKevinWeiss removed the State: stale State: The issue / PR has no activity for >185 days label Aug 12, 2019

chrysn mentioned this pull request Sep 15, 2019

Question: Shell commands via telnet #12233

Closed

MrKevinWeiss mentioned this pull request Oct 18, 2019

riot_pal/dut_shell.py: changed the DutShell to case-insensitive anwser messages RIOT-OS/riot_pal#32

Closed

stale bot added the State: stale State: The issue / PR has no activity for >185 days label Feb 13, 2020

smlng added State: don't stale State: Tell state-bot to ignore this issue and removed State: stale State: The issue / PR has no activity for >185 days labels Feb 17, 2020

MrKevinWeiss mentioned this pull request Sep 17, 2020

[RFC] abstraction layer for test commands #15037

Closed

MrKevinWeiss mentioned this pull request Feb 8, 2021

sys/test_utils/result_output: Initial implementations and API of turo #15950

Merged

MrKevinWeiss added this to the Release 2021.07 milestone Jun 21, 2021

MrKevinWeiss removed this from the Release 2021.07 milestone Jul 15, 2021

MrKevinWeiss closed this Dec 2, 2021

Conversation

MrKevinWeiss commented Dec 18, 2018

Contribution description

Uh oh!

jcarrano 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!

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!

MrKevinWeiss commented Dec 19, 2018

Uh oh!

MichelRottleuthner 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!

danpetry Jan 31, 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!

danpetry commented Jan 31, 2019

Uh oh!

danpetry commented Jan 31, 2019

Uh oh!

MrKevinWeiss commented Feb 1, 2019

Uh oh!

MrKevinWeiss commented Feb 1, 2019

Uh oh!

danpetry commented Feb 4, 2019

Uh oh!

danpetry commented Feb 4, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stale bot commented Aug 10, 2019

Uh oh!

MrKevinWeiss commented Aug 12, 2019

Uh oh!

stale bot commented Feb 13, 2020

Uh oh!

MrKevinWeiss commented Dec 2, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

danpetry Jan 31, 2019 •

edited

Loading

danpetry commented Feb 4, 2019 •

edited

Loading