HTCC-AB02 LoRaWan_APP display conflict

I’m not sure if this is a reasonable comment/request or not, but here goes…

In developing a library for use with several different processors, including Heltec WiFi LoRa 32 and CubeCell Dev-Board Plus, but also other ESP-12 and ESP32 processors, I encountered a conflict with my use of the display library used for the CubeCell Dev-Board Plus.

I originally did what one would normally do:

#include HT_SH1107Wire.h

and created an instance

SH1107Wire  display(0x3c, 500000, SDA, SCL, GEOMETRY_128_64, GPIO10);

then, of course, included the various method calls to send output to the display.

This was all fine, but when I added LoRa code to my sketch, which required:

#include "LoRa_APP.h"

compilation failed with (just the essential part of the relevant message shown below):

/Users/workspace/Documents/Arduino/hardware/CubeCell/CubeCell/libraries/LoRa/src/LoRaWan_APP.cpp:23: multiple definition of `display'

Examination of the LoRaWAN_APP library reveals that it includes, amongst other things, the following ‘public’ definition:

  SH1107Wire  display(0x3c, 500000, SDA, SCL, GEOMETRY_128_64, GPIO10); // addr , freq , i2c group , resolution , rst

I’m not sure that this is appropriate. Shouldn’t an ‘internal’ instance be named something like:

  SH1107Wire  _display(...)

with a preceding underscore, or something similar, to avoid conflict with normal ‘externally’ declared variables and the like?

As it is, a user cannot create an instance of SH1107Wire called ‘display’ in their sketch, even though this would otherwise, if the LoRaWAN_APP library were not loaded, be a perfectly reasonable thing to expect to be able to do. I can’t actually see any reason for a user to believe that display usage would be a necessary component of a LoRaWAN_APP library and thus demand some special level of consideration when including the LoRaWAN_APP library. Indeed, it is not. The inclusion of this definition is conditional on the board being used (i.e. only for the CubeCell_BoardPlus).

But am I missing something?

If not, I wonder if Heltec could consider doing something that would avoid the problem I have described (so that a user doesn’t need to modify the LoRaWAN_APP library every time it is updated). As it is, I have simply deleted the offending lines of code from the LoRaWAN_APP library and all is OK but, with appropriate declaration, I don’t see why the present usage couldn’t be retained without causing external conflicts.

You could try naming it something like:
SH1107Wire sh1107_display(...)

Yes, anything other than ‘display’, on either side of the fence, so to speak, works just fine.

I guess the point was that ‘display’ is used in pretty much every example piece of code relating to a display that I’ve seen. This encourages the use of this name any time an instance of a display class is created. So why use that particular and very commonly used name ‘inside’ a library and risk conflict, especially if the display entity is quite incidental to the purpose of the library in question?

Within my own library, when required, functions simply use a pointer, passed as an argument, to the display entity that has been created in the main sketch, avoiding any potential for this sort of conflict.

This method sounds very good.