SPI defined as enum in OSPIBlockDevice.h

[timIdeaTech]

The following is in OSPIFBlockDevice.h:

enum _mode {
    SPI,
    SOPI,
    DOPI
};

and is causing the following compiler error when I try to instantiate an SPI device when OSPIFBlockDevice.h has been included: error: reference to 'SPI' is ambiguous

I can get around it by using mbed::SPI instead. I'm thinking those enum's should have been prefixed with OSPIBD or similar. The original mbed OS repo also has this problem. If there is no objections I could do the pull request but thought I'd ask here first.

[multiplemonomials]

Ooh good catch on this!

Hmm, what about changing the declaration to:

enum mode {
    SPI,
    SOPI,
    DOPI
};

and moving it inside the OSPIFBlockDevice class? That should make it so they aren't in the global namespace, right?

[timIdeaTech]

The problem is that the _mode value is public because

OSPIFBlockDevice::change_mode(int mode)

is public. Also, naming a global

enum _mode {...

seems like a recipe for collision also, hence why I suggested adding a prefix. Wouldn't ospif_mode and OSPIFMODE[SPI|SOPI|DOPI] or maybe ospif_opi_mode and OPSIF_OPIMODE[SPI|SOPI|DOPI] make sense?

This would follow the the naming conventions of the other enum's _ospif_bderror and _ospif_polaritymode.

[multiplemonomials]

Strictly speaking, it's fine to move it inside the class because you could just make it public in the class, so then people would be able to access it just fine. Also, either way, it's a breaking change -- people will have to change DOPI to either OSPIFBlockDevice::DOPI or OSPIF_MODE_DOPI.

That said, you're right it would have more symmetry with the other types to do it the way you said. So I'll accept either way!

[timIdeaTech]

Ok, I get where and why your going with this. Due to my only moderate experience with C++, I had to go back and reread and get the nuance of the move it inside the ... class suggestion to get rid of the global collision. For this reason I don't have a good feel for the proper convention here.

That said, the convention used in this driver are global driver prefixed enum's so maybe makes sense to stick with it. I'll give this a go...