Hardcoded PinNames PC_8 and PC_12

JammuKekkonen commented 7 years ago

The driver has hard-coded PinNames for DigitalOuts _wakeup and _reset. This causes compilation to fail for targets which don't have these PinNames defined and also makes board configuration impossible for such targets.

JammuKekkonen commented 7 years ago

@JanneKiiskila

JanneKiiskila commented 7 years ago

@sg- @MarceloSalazar @betzw - please take action here, label it right etc.

betzw commented 7 years ago

This is because these pins are expansion board dependent and essentially used with constant states. As I didn't want to blow the constructors with too much parameters I have preferred to make them configurable via the mbed config system.

JanneKiiskila commented 7 years ago

Then we should have an mbed_lib.json which has the board specific definitions and the actual code should just use some "WIFI" -pins, similar to what the SD-card is doing.

https://github.com/ARMmbed/sd-driver

betzw commented 7 years ago

Not sure, in our case the pins in question are expansion board/shield sepcific, and do not dependend on the development board/target.

betzw commented 7 years ago

In the latest version of the driver I have added pins wakeup and reset to the respective constructors and define default values for them. Now you can specify them in both ways, by passing them to the constructor or by configuring them in the mbed config system. Hope this fulfills your requirements. @JammuKekkonen, @JanneKiiskila, @MarceloSalazar, pls. let me know.

JanneKiiskila commented 7 years ago

The constructor input does not resolve this issue directly, in my humble opinion. Issue being that if those aren't defined it will try to use the PC_8 and PC_12. There's plenty of boards that do not have it.

What boards does this Wifi module work with?

If it ONLY works with a Nucleo board, could we add a #ifdef TARGET_NUCLEO -something and thus create protection for these defines? Note - what we're trying to achieve here is to having a working compile without a .mbedignore file. Alternatively some kind of WIFI_X -feature flag, that one would have to define in order to active this.

PC_8 and PC_12 are coming from TARGET definitions, not from the WiFi board itself.

targets/TARGET_STM/TARGET_STM32F0/TARGET_NUCLEO_F030R8/PinNames.h:    PC_8  = 0x28,
targets/TARGET_STM/TARGET_STM32F0/TARGET_NUCLEO_F070RB/PeripheralPins.c:    {PC_8,  PWM_3,  STM_PIN_DATA_EXT(STM_MODE_AF_PP, GPIO_PULLUP, GPIO_AF0_TIM3,  3, 0)},  // TIM3_CH3
targets/TARGET_STM/TARGET_STM32F0/TARGET_NUCLEO_F070RB/PinNames.h:    PC_8  = 0x28,

for example and what we're trying to achieve here is to map the target board pins to the WiFi module side as far as I understand.

It's explicitly these lines of code causing the compilation issue;

https://github.com/ARMmbed/wifi-x-nucleo-idw01m1/blob/6ce14f1f186e844e452a090fc93f2955ffa30ced/SPWFSA01/spwfsa01_at_strings.h#L4-L9

betzw commented 7 years ago

What boards does this Wifi module work with?

If it ONLY works with a Nucleo board, could we add a #ifdef TARGET_NUCLEO -something and thus create protection for these defines?

Theoretically, the X-NUCLEO-IDW01M1 is not only working with Nucleo targets, but if your board doesn't have a Morpho connector you cannot stack the expansion board on top of the development board and need to connect the different pins via wires. On the other hand, the X-NUCLEO_IDW04A1 has an Arduino connector, so you should be able to stack it on top of most targets.

Note - what we're trying to achieve here is to having a working compile without a .mbedignore

Do you mean without a mbed_app.json file? Because in the macros section of this file you could set macros SPWFSAXX_WAKEUP_PIN & SPWFSAXX_WAKEUP_PIN to any value you want. This should also resolve any compilation issue.

JammuKekkonen commented 7 years ago

Hi

Now that all pins are configurable in app config, I think this issue is resolved.

I think the issue @JanneKiiskila is now referring to is that we'd like to have EasyConnect library as 'easy' as possible. One factor in that is that we'd like to keep different .mbedignore file variations and app-level configurations to minimum.

With the default values now in this driver, we would have to either have users of EasyConnect to have different .mbedignore for when they want to use other wifi driver and other when wanting to use this driver or always define the earlier mentioned pins to something that compiles to their target, even if not using this driver through EasyConnect.

Should we open a new issue regarding this? From the top of my head, I don't even have suggestion yet as to where this new issue should be fixed; in this driver or in EasyConnect.

betzw commented 7 years ago

Now I've got it :smile: Well, this can definitely be fixed. Just give me some time.

JanneKiiskila commented 7 years ago

Excellent! You have all the time you want!-)

betzw commented 7 years ago

Pls. take a look at version https://github.com/ARMmbed/wifi-x-nucleo-idw01m1/commit/aca0b940b8d163fd6a37cd5df5a2918ee29401bc.

JanneKiiskila commented 7 years ago

Ok, we should get it integrated to easy-connect and think how we document that.

Are there any plans to reveal the pinout, so that one could connect that module to any board?

betzw commented 7 years ago

on the X-NUCLEO-IDW01M1 expansion board the wakeup and reset pins to use are the one defined for the Morpho connector (i.e. PC_8 and PC_12, you may refer to the description of any Nucleo board - like e.g. the NUCLEO-F401RE - for information regarding the Morpho pin out), while the pins to use on the development board / target can be any pin eligible for DigitalOut.
the X-NUCLEO-IDW04A1 comes with an Arduino connector, so there should be no such issue.

Does this answer your question?

JammuKekkonen commented 7 years ago

Pins are now configurable and defaults will not interfere compilation even through EasyConnect.

ARMmbed / wifi-x-nucleo-idw01m1

Hardcoded PinNames PC_8 and PC_12 #3