[BK7231N] I'm thinking about adding sunrise and sunset to NTP events, anyone interested?

I'm thinking about adding sunrise and sunset to NTP events, anyone interested? Attached is a patch that I haven't tried yet. Any feedback is greatly appreciated.

Rick

The format would be something like this:

// Example: do event every Thursday at sunrise
// addClockEvent sunrise 0x08 12 POWER OFF

The code I used is borrowed heavily from Tasmota, hope there isn't any licensing issue.

Very good job, GPL license is okay. My only concern would be memory size, but we can put that in the obk_config.h as a conditional include. Can you open a pull request on Github so we can review it?

On a code review side, I would suggest some little changes, for a start, I'd think it's better to merge ntp_setLatitude with ntp_setLongitude into one command, you know, we are limited by flash memory, there is no need for two separate command I think

Okay, thanks for the feedback. I folded both latitude and longitude into one command. I was worried about code space too but was very surprised that the whole thing took less than 900 bytes (not counting any possible increase in math library use via all the trig functions it uses, like atanf, sinf, etc). I'll be testing it during the coming week, these kinds of things take a while to test :)

Cheers,

Rick

I've tested the sunrise/sunset algorithm with various cities around the world and it seems to be accurate within +-2 minutes. Not good enough for a space launch blastoff time but maybe good enough for us? There might be an issue with setting it up in the autoexec.bat however. Maybe you have an idea how to fix it. Seems like we might need a "set event when network is live after boot". Also might need to force NTP when the timezone is set instead of doing:

Info:NTP:NTP offset set, wait for next ntp packet to apply changes

The above message means that I can't do this in my autoexec.bat:

PowerSave 1
addRepeatingEvent 10 1 backlog startDriver NTP; ntp_timeZoneOfs -8; NTP_SetLatlong 44.002130 -123.091473; addClockEvent sunrise 0x7f 12 POWER OFF; addClockEvent sunset 0x7f 13 POWER ON

Since the offset won't have taken place when I do "addClockEvent sunrise" and I could (potentially) be in the wrong timezone.

Any ideas about fixing the above is greatly appreciated.

Rick

Hello, are you aware about:

style of syntax?

See random sample script with waitFor here: https://www.elektroda.com/rtvforum/topic4011890-30.html#20822736

EDIT: Altough.... your question may not be strictly related to that, maybe we just need to recalculate the time in-place at the time of command execution... idk, take OLD offset, subtract it from current time and add a new one...?

Added after 44 [minutes]:

EDIT: I did it:
https://github.com/openshwprojects/OpenBK7231...mmit/55e6ca9ac6a5891470b145c015ee6f3094e6096b

>> EDIT: I did it:

Thanks for that! And for the "waitFor NTPState 1" tip. I'll give it a try.

I've been seeing these happen randomly (in groups of 10):
Info:NTP:NTP_CheckForReceive: Error while receiving server's msg

Any ideas?

BTW, my server is:

Info:NTP:Server=217.147.223.78, Time offset=-28800

Why do you think it's a problem? I've a looked at code and it seems normal, it happens when recv fails. It's UDP, it's not reliable, it may fail to receive data sometimes.

>> Why do you think it's a problem? I've looked at the code and it seems normal, it happens when recv fails. It's UDP, it's not reliable, it may fail to receive data sometimes.

I guess maybe I was misled by the "Error" in the message, to me "Error" means there is a problem. But before you change the message verbiage, I have some NTP poll suggestions:
- increase NTP poll time from 1 minute to ~15 minutes, judging from info on the web 1 minute is too fast.
- remove polling (and error msg) each second for 10 seconds if poll fails, just quietly wait until the next polling period. If it fails for 5 polls in a row, issue a warning.

Added after 8 [minutes]:

I've been testing the sunrise/sunset code and it seems to work well. I've attached a patch to version 1.17.308. Please tell me if there are any issues.

Here is an example autoexec on how to use it:

PowerSave 1
startDriver ntp
ntp_timeZoneOfs -8
waitFor NTPState 1
ntp_SetLatLong 44.002130 -123.091473
addClockEvent sunrise 0x7f 12 POWER OFF
addClockEvent sunset 0x7f 13 POWER ON

I have some unrelated questions:

- Do you know where I can get a *real* BK7231N datasheet. The ones in the forum are helpful but not the real thing.
- is there way to put pin config's into autoexec, I'm looking for a way to setup the entire config in one file.

Oh and by the way, I made a mistake in the amount of added space for the sunrise/sunset, it's actually around 11k. Maybe some of that comes from the trig functions that are used. I got this number from looking at the .asm file ending address for 1.17.308 vs my mods.

Thanks for all the help

Rick

Did you get a chance to look at the patch? If you think the 11k is too much I could #ifdef it with a config setting.

Rick

Sorry for the late reply. I've checked the code, it's good, but why are you adding new fields to config structure? If config size changes between versions, then downgrade will not be possible.
If you really need to use config structure, maybe reuse those fields:


Or maybe just use global variables and expect user to set them in autoexec.bat every time?

The BK7231 information on our forum is all we know.

You can use setPinRole to set pins via autoexec.bat : https://github.com/openshwprojects/OpenBK7231T_App/blob/main/docs/commands.md

All sunset/sunrise code should be within #define put in obk_config.h

> Sorry for the late reply.
No problem.

> I've checked the code, it's good, but why are you adding new fields to the config structure? If the config size changes between versions, then downgrade will not be possible.
> If you really need to use the config structure, maybe reuse those fields:
> Code: c Expand Select all Copy to clipboard

I did it that way because I thought it was the "right" way to do it. I don't really know this code that well, so how about you tell me the way you think it should be done, and I'll do it.

> The BK7231 information on our forum is all we know.
> You can use setPinRole to set pins via autoexec.bat: https://github.com/openshwprojects/OpenBK7231T_App/blob/main/docs/commands.md

Thanks much for the info.

> All sunset/sunrise code should be within #define put in obk_config.h

Okay, I'll work on that.

If you are unsure about config save, then just let's use global variables and assume that it should be put in autoexec.bat and set on every startup that way

Is sunrise and sunset working?
Would be great!

Yes, what is the state of that? Can I merge?

Still testing it. I should have something later today.

Got a question though..

What's the correct way to send you an example autoexec.bat and changes to https://github.com/openshwprojects/OpenBK7231T_App/blob/main/docs/commands.md?

commands.md is autogenerated from JSON comments in code, do you have a Github account?

Thank you Rickbronson for this!
Wow that is 11K (byte?), sorry.

>>20852637
I do have a GitHub account: github.com/rickbronson

Can you open a pull request with your code changes?
Just move the variables from config to globals (or statics), and let's just assume they will be set every session through autoexec.bat

Never done a git pull request but would like to try it. Before I do that, please take a quick look at the patch and see if you notice anything you'd like changed.

Here is the patch.

It's 99% okay and you can open pull request, however, I forgot to mention, that if you want to add autoexec to docs, you do not need to modify the MD file, you just modify the source json:
https://github.com/openshwprojects/OpenBK7231T_App/blob/main/docs/json/autoexecExamples.json
and put autoexec there:
https://github.com/openshwprojects/OpenBK7231T_App/tree/main/docs/autoexecs
then if you run:

it will generate new MDs based on that info

But again, it's not a problem, it's just documentation related, the code seems good now

You'll have to excuse my ignorance of git. I've only used it a little. Here is what I get (on Debian):

rick@laptop:~/boards/tuya-temp-humidity/OpenBK7231T_App$ git push -u origin sunevent_branch
Username for 'https://github.com': rickbronson
Password for 'https://rickbronson@github.com':
remote: Permission to openshwprojects/OpenBK7231T_App.git denied to rickbronson.
fatal: unable to access 'https://github.com/openshwprojects/OpenBK7231T_App/': The requested URL returned error: 403
r

I haven't used git command line in a very long time, but don't you need first to push to your local cloned branch, like:

(I modified URL, so it's on your account now) and then open the pull request?

These days I am using Github GUI 100% time, it is much easier, maybe you can try it out as well.

Yeah, I tried the GUI first but found it very confusing.
I went to: https://github.com/openshwprojects/OpenBK7231T_App/pulls
and hit "New pull request" and then, very confusingly, it went to a "Compare changes" page. There was a "create pull request" on that page but it was grayed out. Here is the latest patch, just FYI

Added after 1 [minutes]:

Patch

>>20855929
Okay, I pushed my changes to: https://github.com/rickbronson/OpenBK7231T_App
But I'm not sure where to go next...

Both Github GUI and Github itself should propose making you a pull request from your branch to parent repository, I think

Does sunrise/set work? I can try it out if experimental

Added after 1 [minutes]: