rszimm / sprinklers_pi

Sprinkling System Control Program for the Raspberry Pi
GNU General Public License v2.0
309 stars 100 forks source link

Schedule name length #185

Open tidharmor opened 2 years ago

tidharmor commented 2 years ago

I tried changing my schedules names, and after changing 2 schedules successfully, the 3rd change caused all schedules to disappear. The Schedules item on the main menu still showed it has 3 schedules, but when I entered the Schedules page to edit them none were displayed. This has happened before already and that time I just reset the system and started over (it happened when I first set up the system). This time I managed to return it to display the schedules by editing the settings file with a hex editor and deleting some characters from the schedule with the long name. This is the log of the relevant time when I changed the schedule names, specifically at 2021/09/12 08:32:32 when schedule id 2 was changed. log.txt My schedule names are in Hebrew which might somehow cause this issue.

Edit: I just browsed the code and saw that in the Schedule class, the name field is char[20], which probably explains the problem. Since I enter the schedule name in Hebrew, each letter is actually 2 bytes, so any name over 10 letters exceeds the array boundary. I don't know anything about the EEPROM size limits so I don't know if the name array can be made longer, but probably adding a check for the length when saving would prevent this from causing any problems. If I'll be able to find some more time I might try to fix the issue and submit a pull request.

nhorvath commented 2 years ago

Could you tell me how long the labels were, and how many characters you reduced it to before it worked? Thanks.

On Sun, Sep 12, 2021 at 5:24 AM tidharmor @.***> wrote:

I tried changing my schedules names, and after changing 2 schedules successfully, the 3rd change caused all schedules to disappear. The Schedules item on the main menu still showed it has 3 schedules, but when I entered the Schedules page to edit them none were displayed. This has happened before already and that time I just reset the system and started over (it happened when I first set up the system). This time I managed to return it to display the schedules by editing the settings file with a hex editor and deleting some characters from the schedule with the long name. This is the log of the relevant time when I changed the schedule names, specifically at 2021/09/12 08:32:32 when schedule id 2 was changed. log.txt https://github.com/rszimm/sprinklers_pi/files/7149330/log.txt

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/rszimm/sprinklers_pi/issues/185, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABIJXBHR2B2QFOCWUQSH2TUBRWTTANCNFSM5D34XJHQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

tidharmor commented 2 years ago

The name I set was "טפטפות קיץ", so it's 10 letters (including the space), which I guess would be 20 bytes since it's Hebrew. (There might be a leading or trailing space, so it may be even longer) I edited the settings file with a hex editor and just deleted a few characters from where I saw the string, it ended up 6 letters after I edited it. I'm attaching a zip file with 2 settings files: one which has the long name, and the other after I shortened it. settings.zip

nhorvath commented 2 years ago

Ah that's probably the bug. It probably checks strlen not how much space it takes up so unicode overflows the allocated memory in the eeprom.

On Sun, Sep 12, 2021 at 9:39 AM tidharmor @.***> wrote:

The name I set was "טפטפות קיץ", so it's 10 letters (including the space), which I guess would be 20 bytes since it's Hebrew. (There might be a leading or trailing space, so it may be even longer) I edited the settings file with a hex editor and just deleted a few characters from where I saw the string, it ended up 6 letters after I edited it. I'm attaching a zip file with 2 settings files: one which has the long name, and the other after I shortened it. settings.zip https://github.com/rszimm/sprinklers_pi/files/7149648/settings.zip

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/rszimm/sprinklers_pi/issues/185#issuecomment-917638242, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABIJXEGTQSCXCPZHIBRFELUBSUPNANCNFSM5D34XJHQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.