Why running the same test on 4 PWMs ? about ci-test-shield HOT 10 CLOSED

All, please go take a look at https://gist.github.com/BlackstoneEngineering/e3db036272daa2e5424076fbca8b9d43 . I feel it may be more productive to collaborate on the redesign for more modularity than try to patch it in here.

from ci-test-shield.

@LMESTM ,

Here is my explanation for why things the way they are (for now):

1. The software was written to test boards that are compliant with the Arduino R3 header hardware spec, that is the spec mbed recommends to its partners and the one we see the most used. That said we are aware that usually most vendors do not implement the full spec, so we are working on a possible work around (see #3 below)
2. If a board has more than 1 PWM is it necessary to check all of them, I have seen multiple boards pass with PWM0 working but PWM 2-4 do not work because the software port was botched. Testing all the PWM's (if they are available) should be done to prevent this. We wrote these tests into the CI Test shield because it is necessary to test everything possible and I would prefer to automate it all than have to do it by hand.
3. We are aware that some boards may not have PWM2-4, or some subset there in. I am working with the partnership team on a solution. For the moment, if your board does not support PWM2-4 then the failure is to be expected. Simply tell your partner manager this and make sure your pinout diagram reflects this an you will be fine. I know its not an optimal solution, and we're working on a better way.

As to the suggestion for adding PWMIN_0-4 defines to the mbed_app.json file, I will leave that decision to @0xc0170 as he is the maintainer. The intial draft was done with the pins hard coded to avoid clutter in mbed_app.json file and because the CI Test shield has wire traces that loopback to those pins (again, per the Arduino R3 header spec). So adding another perameter to the mbed_app.json file would not necessarily fix things, because if you wanted to change where the pins looped back you would need to physically modify the CI Test shield. If you have to modify the shield to pass your tests then you have to supply us with 10x of those custom boards and submit the software in a seperate branch so we can add it to our CI test farm. (We do not allow fly wires on the CI Test Farm as things break too easily)

from ci-test-shield.

@BlackstoneEngineering
thanks for you answer. I understand the background.
Reducing the number of PWMs to tests and adding defines in mbed_app.json file, would also allow various work-arounds, like picking up 1 or 2 available PWMs out of the 4 possibilities on ci-test-shields.
Cheers
Laurent

from ci-test-shield.

@0xc0170 we should add this to the list of things to consider in the next cycle. Either adding configuration to the json file or making 'levels' of compliance, where the top level tests everything.

@LMESTM just to confirm, your use case is that only say PWM1 and 2 are hooked up, and not PWM 3 or 4, so you're getting failures for PWM 3/4 when your board doesnt support them, correct?

from ci-test-shield.

@BlackstoneEngineering yes indeed. For instance https://developer.mbed.org/platforms/ST-Nucleo-F410RB/ has PWMs on D2, D7 and D8, but not on other [D2 - D9] pins

from ci-test-shield.

@BlackstoneEngineering yes indeed. For instance https://developer.mbed.org/platforms/ST-Nucleo-F410RB/ has PWMs on D2, D7 and D8, but not on other [D2 - D9] pins

@LMESTM Can you send a patch with a proposal to have pins configurable per target? Or propose it here how ti would look?

from ci-test-shield.

@LMESTM Can you send a patch with a proposal to have pins configurable per target? Or propose it here how it would look?

The principle is very basic. Mainly define the digital input for checking pwm in mbed_app.json file e.g. PWMIN_0, althgouh I'm not so happy with this misleading name)
"PWM_0": "D2",
"PWMIN_0": "D3",
"PWM_1": "D4",
"PWMIN_1": "D4",
"PWM_2": "D5",
"PWMIN_2": "D6",
"PWM_3": "D7",
"PWMIN_3": "D8",

Then the PWM.cpp would rely on it.

In case a platform does not have full Arduino compatibility, it would be able to redefine any of the pins to match the HW in mbed_app.json.

from ci-test-shield.

I feel it may be more productive to collaborate on the redesign for more modularity than try to patch it in here.

Agree. That led me to an idea, it might be better to use git repo than gist (for review). If we can send this to this repo as PR (RFC type, docs folder for instance, once we all agree on the form, it could become doc in this repository), and start discussing it here, see how the docs is changing .

The principle is very basic. Mainly define the digital input for checking pwm in mbed_app.json file e.g. PWMIN_0, althgouh I'm not so happy with this misleading name)

Thanks, I had something similar on mind. Would it make sense to have one test for arduino compability (that one would fail in this case as not all pins are PWM capable that should be) but PWM test would not fail as it would take input from config file as you proposed above (=PWM is working as it was tested on some pins, but not full set in some cases). Then if I read the report. I would see this platform does not conform to some assumptions about pinout, but the peripheral driver is all good. This would not result in misleading results (red test just because one pin is not wired, etc).

from ci-test-shield.

Thanks, I had something similar on mind. Would it make sense to have one test for arduino compability

yes makes sense.
Note that I don't see where Arduino mandates that all D2-D9 pins can support HW PWMs.
In Arduino, PWMs are often considered as the output of a call to "analogWrite(pin, dutyCycle),". Basically Arduino "PWMs" also include pseudo analog emulation, or even emulated PWMs with GPIOs, not HW timers based PWMs. In MBED, as I see it, PwmOut are HW PWMs, unless we implement SW PWM in MBED ...

from ci-test-shield.

This is ok now - so closing ...

from ci-test-shield.