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

[P118] Make CS pin configurable, some refactoring #3889

Merged

Conversation

tonhuisman
Copy link
Contributor

@tonhuisman tonhuisman commented Dec 27, 2021

Features/improvements:

  • Make the CS pin configurable
  • Resolve some compiler warnings
  • Leave SPI initialization to ESPEasy core
  • Refactor to use Plugin_data_struct to enable multi-instance and save some memory
  • Format source code using Uncrustify
  • Add support for Orcon devices, taken (with permission) from [P118] Add Orcon functionality #4099 by @nl0pvm
  • Moved to Collection E because of the size increase.
  • Add CLIMATE builds and move this plugin there

TODO:

  • Test on ESP8266 (confirmed)
  • Test on ESP32
  • Restore plugin in ESP32 builds
  • Test Orcon commands (help requested) (confirmed)

@svollebregt
Copy link
Contributor

Shall I already test this on my ESP8266+CC1101 or are you planning to add the Plugin_data_struct soon?

@tonhuisman
Copy link
Contributor Author

Shall I already test this on my ESP8266+CC1101 or are you planning to add the Plugin_data_struct soon?

I'm close to committing those changes as well, not sure I'll manage that today, could be tomorrow end of morning.

lib/Itho/CC1101.h Outdated Show resolved Hide resolved
src/_P118_Itho.ino Outdated Show resolved Hide resolved
src/_P118_Itho.ino Outdated Show resolved Hide resolved
@svollebregt
Copy link
Contributor

I downloaded the branch from @tonhuisman and build it using Atom. However, when I flash my ESP either OTA or using the ESP Easy Flasher my device will not boot anymore.

Below the output of the serial log:
;d d£ƒ| îlα|♥♀♦♦î♦d∞♦#<╟├♥Σ↕█{Æ#ä♀c─≤on▀lo'▄πΣ♦#∟8Σ╟$;dsl8√'α►♥♦♦é♦d♀─£♀♀♦c♀nΓ|♥Σdç♦╟c─√go∩ lîÅd♥╪←←'o♦d♥♥gsçôÆn♦♀#♥l♥☼s██'♀♦c♥l♥£#♦♦ç∟♥¢l;¢♥ⁿén▄♥¬U83 : Info :

INIT : Booting version: pygit2_not_installed (ESP82xx Core 2843a5ac, NONOS SDK 2.2.2-dev(38a443e), LWIP: 2.1.2 PUYA support)
84 : Info : INIT : Free RAM:31872
85 : Info : INIT : Manual Reboot #10 - Restart Reason: External System
86 : Info : FS : Mounting...
111 : Info`

@tonhuisman
Copy link
Contributor Author

tonhuisman commented Jan 1, 2022

Hmm, there is always a risk when refactoring that something breaks 🤔
If you download the Github Actions build from here, and extract a TEST_D build that matches your ESP, does that work any better?
Flashing a working build, then disabling the P118 task before upgrading, and then do the upgrade should at least get you a running unit. Then from Tools page you could execute taskenable,<tasknumber> to try it (but it might still fail/crash, but you won't go into a boot-loop as the task is then still disabled at boot).

@svollebregt
Copy link
Contributor

First disabling P118 and then flashing it with that build did result in a successful boot. But it immediately crashes when I enable the task associated with P118. In the weblog which I had open in another tab nothing is visible.

@tonhuisman
Copy link
Contributor Author

Any crashlog is only really visible in the serial log, the weblog can't be updated anymore if the unit is crashing.
So I have some investigation to do. If you could get that crashlog, that would be very helpful.

@tonhuisman
Copy link
Contributor Author

tonhuisman commented Jan 3, 2022

@svollebregt I've tried to move some bits, and reverted the disable/enable interrupt logic I accidentally introduced when moving the code to Plugin_data_struct.
If, during testing, you could watch the serial log, then you'd have a better look at the actual crash information (both before and after the big hex-dump it usually outputs), if that still happens, then please post those parts of the log here.
NB: Download from here, for this updated build.

@svollebregt
Copy link
Contributor

svollebregt commented Jan 8, 2022

I connected the device to my PC and opened a serial monitor with your latest commit loaded, with the serial output set to Debug Dev. When I enable the plugin I see the following messages:

152783 : Info : HTTP: taskenable,2
152787 : Debug : Command: taskenable
152791 : Info : INIT PLUGIN_118
152792 : Info : Extra Settings PLUGIN_118 loaded

No crash dump is generated. I tried this using the serial monitor of Atom, Arduino IDE, and MobaXterm. Do I need a special tool to see a crash log, or does the ESP simply not crash but just freezes?

@TD-er
Copy link
Member

TD-er commented Jan 8, 2022

To decode the crash dump, you need the ESP connected to the IDE (VS code, not sure if Atom allows the stackdecoder to run) and run "monitor" from the PlatformIO menu.
You also need to have the code loaded and the right .elf file, so it is best to compile it and upload it from your PC to make sure you have the correct .elf file related to the code you're running.

@tonhuisman
Copy link
Contributor Author

The $64000,- question: Does it still crash? (I can't determine that from your comment...)

@TD-er
Copy link
Member

TD-er commented Jan 8, 2022

The $64000,- question: Does it still crash? (I can't determine that from your comment...)

I wouldn't be surprised if it did...
See also why I disabled it here (temporary) for the IDF4.4 PR as just having it present in that build will cause crashes/boot loops on the ESP32.
There is something fishy with the callback functions (or attached to interrupts)

@tonhuisman
Copy link
Contributor Author

Well, there is another observation: If the plugin is started when there is no hardware connected, it just hangs forever. Not even the WDT kicks in to revive the unit. At least that's what I get when enabling the plugin, and that's exactly the same behavior I got before I started the refactoring.
Not sure if @svollebregt is testing with the hardware connected or not, but this plugin won't work without it. (We might need to note that somewhere).

@tonhuisman
Copy link
Contributor Author

And this is the culprit: https://github.com/letscontrolit/ESPEasy/blob/mega/lib/Itho/CC1101.cpp#L31-L34
That explains why the WDT isn't kicking in.
It is called on each writeCommand() and writeRegister() call, and a few other places...

@svollebregt
Copy link
Contributor

I was testing it with all hardware attached (minus a CO2 sensor as it uses the serial port, disabled that plugin). I switched to VS Studio, and performed a successful build and monitor. Output on the terminal is the same, seems it hangs and does not crash.

@tonhuisman
Copy link
Contributor Author

I see a couple of while loops (at least in the CC1101 library) that have the potential to last 'quite long' (as in 'forever'), if an expected state isn't reached.
I've tried to fix the blocking loop I referenced, but it is not enough. Without the hardware available it will be quite hard to debug, so I'll order such a board to dive deeper into this. Until that arrives, this PR will go on hold, unless of course you are able to the debugging 😉
I've also changed the declaration of the interrupt handler, according to another example, where the ICACHE_RAM_ATTR attribute is applied to the function declaration, and not to the implementation. I'll commit that change in a minute.

@One35SEven
Copy link

Maybe the original author of the Orcon part, @nl0pvm , can assist here? Edit: You could send the command state,1111 from the Tools page while the ventilation unit is still in learning mode, that's the way to connect to an ITHO unit, might also work for Orcon.

Tried state,1111, but doesn't seem to work. Will see if i can find more in the tweakers.net forums. Hopefully @nl0pvm can assist.

Don't see any difference when turning on/off RF Debugging

That should show some more logging in the log output, either via serial or web-log in the UI. Edit: It will show log lines starting with ITHO: Received from ID: then include the ID from the device that sent it, and the raw command (numeric code) sent.
With debugging off, i also see the "ITHO: Received from ID: << ID >> messages.

@tonhuisman
Copy link
Contributor Author

With debugging off, i also see the "ITHO: Received from ID: << ID >> messages.

What build are you running, a Release build of ESPEasy, of downloaded from this PR's Actions build? That may be the difference, as I tried to modify that behavior, AFAICS...

@One35SEven
Copy link

What build are you running, a Release build of ESPEasy, of downloaded from this PR's Actions build? That may be the difference, as I tried to modify that behavior, AFAICS...

Didn't realize there was a new version (not that well versed in github)

Was running:
Build: ESP_Easy_mega_20220824_collection_E_ESP32s2_4M316k Aug 24 2022
Upgraded to last night:
Build: ESP_Easy_mega_20220914_collection_E_ESP32s2_4M316k Sep 14 2022

I don't see a difference when enabling RF debug.

`
Enable RF DEBUG log: disabled
Enable minimal RF INFO log: disabled

3008819: WD : Uptime 50 ConnectFailures 0 FreeMem 111232 WiFiStatus WL_CONNECTED 3 ESPeasy internal wifi status: Conn. IP Init
3038819: WD : Uptime 51 ConnectFailures 0 FreeMem 111380 WiFiStatus WL_CONNECTED 3 ESPeasy internal wifi status: Conn. IP Init

Enable RF DEBUG log: disabled
Enable minimal RF INFO log: enabled

3157320: ITHO: Received from ID: 33,d2,9; raw cmd: 0
3158819: WD : Uptime 53 ConnectFailures 0 FreeMem 111160 WiFiStatus WL_CONNECTED 3 ESPeasy internal wifi status: Conn. IP Init
3167040: ITHO: Received from ID: 68,52,15; raw cmd: 0
3177000: ITHO: Received from ID: 78,18,be; raw cmd: 0
3188819: WD : Uptime 53 ConnectFailures 0 FreeMem 111316 WiFiStatus WL_CONNECTED 3 ESPeasy internal wifi status: Conn. IP Init
3196661: ITHO: Received from ID: 9a,d2,4; raw cmd: 0

Enable RF DEBUG log: enabled
Enable minimal RF INFO log: enabled

3334228: ITHO: Received from ID: 5,26,58; raw cmd: 0
3338820: WD : Uptime 56 ConnectFailures 0 FreeMem 111436 WiFiStatus WL_CONNECTED 3 ESPeasy internal wifi status: Conn. IP Init
3346925: ITHO: Received from ID: 82,6a,f2; raw cmd: 0
3368819: WD : Uptime 56 ConnectFailures 0 FreeMem 111440 WiFiStatus WL_CONNECTED 3 ESPeasy internal wifi status: Conn. IP Init
3368917: ITHO: Received from ID: 78,18,be; raw cmd: 0
3370705: ITHO: Received from ID: 1a,20,14; raw cmd: 0

`

@One35SEven
Copy link

For my Orcon Fan. It seems every once in a while I see the cmd: 101 in the debug log. And the ID that follows that might be the return "OK" from my fan. All not as consistent as for the ITHO remotes.
But coundn't fill out the hex letters a,b,c,d,f in the device ID fields for the Device ID byte 1, 2 3 fields, only digits and the e.
In my case i needed an a.

@tonhuisman
Copy link
Contributor Author

But coundn't fill out the hex letters a,b,c,d,f in the device ID fields for the Device ID byte 1, 2 3 fields, only digits and the e.
In my case i needed an a.

Those input fields expect decimal input, but the logged numbers are hexadecimal. You can use the Windows calculator in Developer/Programmer mode to enter each hex value (separately) to see what the decimal value is and fill in those 3 values.

@One35SEven
Copy link

Those input fields expect decimal input, but the logged numbers are hexadecimal. You can use the Windows calculator in Developer/Programmer mode to enter each hex value (separately) to see what the decimal value is and fill in those 3 values.

Ah; Was thinking about the Unit IDs that are in hex. Thanks will use decimal values for Device ID fields.

@tonhuisman
Copy link
Contributor Author

I don't see a difference when enabling RF debug.

When this option is enabled, and also the Log level is set for Debug, some extra logging is output. This requires the Debug log level to be available, but that's not in the Collection builds...
I'll hide the UI option when it's not useful to be set.

@TD-er
Copy link
Member

TD-er commented Sep 19, 2022

This one adds over 5k to the build size.
Seems a lot when just looking at the code.

@One35SEven
Copy link

I have tested with my Orcon FAN and it seems to work. Now trying with 2 CC1101 chips connected to the same ESP32S2.

One for Itho and one for Orcon fan. After some testing and configuring Home assistant 2 questions:

  • How to give command to the 2nd device via HTTP? (http://<IP ESPEASY/control?cmd=State,1 only works for first device)
  • It seems that then giving a command via MQTT to the 2nd device (ORCON in my case), the State for the 1st device gets updated in stead of the State for the 2nd device. Might this be a bug?

Also I have some suggestions for the documentation page, where can I put those? Couldn't find where in github.

@TD-er
Copy link
Member

TD-er commented Sep 22, 2022

  • How to give command to the 2nd device via HTTP? (http://<IP ESPEASY/control?cmd=State,1 only works for first device)

See documentation on multiple instances of the same plugin

@tonhuisman
Copy link
Contributor Author

Also I have some suggestions for the documentation page, where can I put those? Couldn't find where in github.

You can email that to me (see GH profile), or place your remarks here, and I will include it.

The docs/source folder of the repository holds the documentation in ReStructured Text/Sphinx format.

@One35SEven
Copy link

Not sure if I can still report/ask something here, but I 'm running into an issue. Had to wait for new cc1101 chip.
I now have 2 cc1101 chips connected to my esp32. Connected using information on this page on how to connect multiple SPI's.
So 2 tasks configured, one per cc1101 chip.

Now i'm running into the following. When sending commands via http directly everything works like i would expect.
When sending: http:///control?cmd=[1].State,102, the state of task 1 gets updated on the devices page.
When sending http:///control?cmd=[2].State,2, the state of task 2 gets updated on the devices page.

But when I use MQTT, something is wrong
When I send to the topic for device 1 the state of task 1 gets updated on the devices page.
When i send to the topic for device 2 also the state of task 1 gets updated on the devices page.

So something seems wrong here.

@tonhuisman
Copy link
Contributor Author

But when I use MQTT, something is wrong When I send to the topic for device 1 the state of task 1 gets updated on the devices page. When i send to the topic for device 2 also the state of task 1 gets updated on the devices page.

So something seems wrong here.

That is most likely not related to this plugin. What is the exact topic you send via MQTT?

@One35SEven
Copy link

But when I use MQTT, something is wrong When I send to the topic for device 1 the state of task 1 gets updated on the devices page. When i send to the topic for device 2 also the state of task 1 gets updated on the devices page.
So something seems wrong here.

That is most likely not related to this plugin. What is the exact topic you send via MQTT?

esp-fan/FAN01/cmd = State 1
esp-fan/FAN02/cmd = State 101

@tonhuisman
Copy link
Contributor Author

Ah, the topic isn't used to address a specific task, as it's not easy to determine what part of the topic relates to a task, if any, so assuming the tasks are named fan01 and fan02, you should use fan01.state,1 and fan02.state,101 for the values, similar to the http commands (though you used the tasknumbers there, you can also use the taskname as a prefix. The square braces are optional)

@One35SEven
Copy link

Ah, the topic isn't used to address a specific task, as it's not easy to determine what part of the topic relates to a task, if any, so assuming the tasks are named fan01 and fan02, you should use fan01.state,1 and fan02.state,101 for the values, similar to the http commands (though you used the tasknumbers there, you can also use the taskname as a prefix. The square braces are optional)

Thanks. This works. Although didn't expect to be able to send commands to either cmd-topic.
So sending a value for FAN01 to the topic for FAN02 and vice versa works. Below gets same result:
esp-fan/FAN01/cmd = FAN02.state,100
esp-fan/FAN02/cmd = FAN02.state,100

Still working on right Home assistant configuration not happy with it yet), but when i have everything complete will send you a mail with updates for the documentation.
(Home Assistant it seems can't handle a fan without an off setting, the templates seem to be for a room fan in stead of a central ventilation system).

@TD-er
Copy link
Member

TD-er commented Oct 14, 2022

@One35SEven Have you seen this section in the docs? Home Assistant (openHAB) MQTT - Command Handling

@TD-er TD-er merged commit 7c95e5e into letscontrolit:mega Oct 16, 2022
@tonhuisman tonhuisman deleted the feature/P118-select-cs-and-refactoring branch October 16, 2022 13:13
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

Successfully merging this pull request may close these issues.

None yet

4 participants