Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unpredictable behavior in a specific case of unused decimal point pins caused by an array out of bounds condition #109

Open
6v6gt-duino opened this issue Mar 3, 2024 · 3 comments

Comments

@6v6gt-duino
Copy link

6v6gt-duino commented Mar 3, 2024

Unpredictable behavior occurs if only seven segments pins (that is no decimal point pins) are defined in the array segmentPins[] in the call to the begin() method but the parameter disableDecPoint is omitted.

This could be the case when the decimal point pins of the seven segment displays are not used.

The problem is that the default value of disableDecPoint is false. This leads to the library treating the number of elements in segmentPins[] as 8 even though only 7 elements are present. When the array is later scanned an attempt is made to read the non-existent element number 8 leading to this unpredictable behavior [array out of bounds].

In the specific case recorded at https://forum.arduino.cc/t/2-digit-display-with-sevseg-library-a-segment-staying-on/1230896 the effect of this error was that an unwanted segment on a display remained lit.

@DeanIsMe
Copy link
Owner

DeanIsMe commented Mar 4, 2024

That makes sense. With this setup, SevSeg reads in 8 pin numbers, where the 8th pin number is not defined.
I could and probably should make the interface more strict using std::array or variadic templates or a setup struct, or a sizeof(segmentPins) argument...
But I likely won't get the chance soon.
Unfortunately most of these would be breaking changes to the begin() function. The way that I set it up is not too flexible.

@6v6gt-duino
Copy link
Author

6v6gt-duino commented Mar 4, 2024

Many thanks for looking at this so quickly.
In retrospect, It is probably best to treat this case as a user error and maybe something to be addressed in a future release by enhanced input validation. I could imagine adding a new format begin() method signature but retaining the old format one for compatibility with old code. Anyway, if the user's starting point had been one of the up to date examples in the library, this should not have happened. It is, of course, not optimal that inconsistent user input gives rise to internal corruption errors (array overflow etc.) but then again as long as the rules are followed, everything should work.

There are, unfortunately, code samples circulating which are misleading, for example this implying that all works when only seven segment pins are defined and without reference to a disableDecPoint parameter : https://forum.arduino.cc/t/how-to-use-a-2-digit-7-segment-display-with-sevseg-h-library/644014. I'll deal with that one specifically by requesting that the tread is reopened and adding a comment.

If you wish you can mark this as closed.

@DeanIsMe
Copy link
Owner

DeanIsMe commented Mar 5, 2024 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants