Typo in Arduino.h constant TIM_DIV265

[paccerdk]

I believe line 90 in Arduino/cores/esp8266/Arduino.h (Master) should be:
#define TIM_DIV256 3 //312.5Khz (1 tick = 3.2us - 26843542.4 us max)
and not:
#define TIM_DIV265 3 //312.5Khz (1 tick = 3.2us - 26843542.4 us max)
which it is, currently. (256 and not 265)

I did not submit a pull request or anything, since it might break stuff if corrected.

https://github.com/esp8266/Arduino/blob/master/cores/esp8266/Arduino.h#L90

[devyte]

@igrr I think I saw a discussion about this somewhere, but I can't find anything in this repo. Do you know anything about it? If memory serves, the discussion was like @paccerdk says: should the typo be fixed or kept for compatibility to not break people's code.
I think this should be fixed. Having code out there depend on a bug or typo is never a good idea (history lessons). At some point, code that currently depends on it may migrate to a newer release or git. At that point it won't compile, they will search for the reason, and they can correct any (wrong) dependencies then.

[igrr]

Thinking of something like

enum {
   TIM_DIV1 = 1,
   TIM_DIV16 = 2,
   TIM_DIV256 = 3,
   TIM_DIV265 __attribute__((deprecated)) = TIM_DIV256
};

and then remove TIM_DIV265 in one of the subsequent versions, giving e.g. library authors some time to update the code.

[devyte]

Relevant: PR #2695

[devyte]

@igrr I tried the enum deprecation that you suggested, and it didn't build. A bit of google says that gcc >= 6 is needed.
I tried a couple of variations, but they didn't work or didn't trigger a warning.
I created #4318 for now. If there's a way that works with our gcc, please let me know and I'll give it a shot. Otherwise, I suggest merging the PR now, and deprecating once we move to a newer gcc.
Also, I suggest removing the wrong 265 for milestone 3.0.0 (open new issue?).

[d-a-v]

with a const that would be possible (link) with our gcc.

[devyte]

I tried something similar to:
static const TIM_DIV265 = TIM_DIV256;
in Arduino.h, then this in a sketch:
const int tt = TIM_DIV265;

The warning didn't trigger, so I left it out. Not sure if I missed something or what.
In any case, I think a static const in Arduino.h would mean 4 bytes extra in every translation unit that includes it. Not sure it's worth it.

[d-a-v]

For the warning you need deprecated
extern "C" const int TIM_DIV265 __attribute__((deprecated, weak)) = TIM_DIV256;
(weak because we are in a .h)

The linker may remove the int if it not used. I did not check that.

[devyte]

@d-a-v I meant I tried this in Arduino.h
static const int TIM_DIV265 attribute((deprecated, weak)) = TIM_DIV256;

and:
const int tt = TIM_DIV265;

and the warning didn't trigger.