search

Cereal2nd / velbus-aio

Velbus Asyncio
Apache License 2.0
15 stars 10 forks source link

Sequential scan #95

Closed sidlgor closed 3 months ago

sidlgor commented 6 months ago

Scans modules sequential

As discussed on Velbus forum ...

Cereal2nd commented 6 months ago

is this tested?

sidlgor commented 6 months ago

No this is not tested at all. Actually I think there is still a bug with a bad placed else at line 90 of handler.py. I don't know how te test integration in hass

sidlgor commented 5 months ago

Could you make some time to look at the code? Is there anything I can do ... just let me know ...

------ Original Message ------ From "Maikel Punie" @.> To "Cereal2nd/velbus-aio" @.> Cc "sidlgor" @.>; "Author" @.> Date 01/04/2024 17:07:59 Subject Re: [Cereal2nd/velbus-aio] Sequential scan (PR #95)

is this tested?

— Reply to this email directly, view it on GitHub https://github.com/Cereal2nd/velbus-aio/pull/95#issuecomment-2029912190, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACBL57UZUZUV3JSFFZO2H5LY3FZ47AVCNFSM6AAAAABFRQSTFWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMRZHEYTEMJZGA. You are receiving this because you authored the thread.Message ID: @.***>

Cereal2nd commented 5 months ago

i looked at it, but this is still a lot of work todo.

there are other priorities to be done before i can start working on this.

sidlgor commented 5 months ago

For my info: Is there more code to change? Did I miss other places than "scan" to adapt or is the work involving setting up a test environment?

If the work involved is in the testing, I wonder if I can just load the new code from git. The changes are quite small refactorings so that I should be able to test without setting up a full debugging environment (just adding some log messages should do it). This would also mean that I can run the forked version until you got time to check and do the full integration (I am totally blocked now).

If I missed other places than scan to adapt and its not to complicated to explain I am always willing to do my best to help ...

I am running HAS Docker with HACS, SSH and Visual Studio Code integration

------ Original Message ------ From "Maikel Punie" @.> To "Cereal2nd/velbus-aio" @.> Cc "sidlgor" @.>; "Author" @.> Date 06/04/2024 09:58:21 Subject Re: [Cereal2nd/velbus-aio] Sequential scan (PR #95)

i looked at it, but this is still a lot of work todo.

there are other priorities to be done before i can start working on this.

— Reply to this email directly, view it on GitHub https://github.com/Cereal2nd/velbus-aio/pull/95#issuecomment-2041009151, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACBL57X5PGDZNQ2R4ZKCAULY36TJ3AVCNFSM6AAAAABFRQSTFWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANBRGAYDSMJVGE. You are receiving this because you authored the thread.Message ID: @.***>

Cereal2nd commented 5 months ago

you can just test this stand alone, without hass, clone the velbusaio repo and work there

there are some example scripts i use

On Sun, Apr 7, 2024 at 5:41 AM sidlgor @.***> wrote:

For my info: Is there more code to change? Did I miss other places than "scan" to adapt or is the work involving setting up a test environment?

If the work involved is in the testing, I wonder if I can just load the new code from git. The changes are quite small refactorings so that I should be able to test without setting up a full debugging environment (just adding some log messages should do it). This would also mean that I can run the forked version until you got time to check and do the full integration (I am totally blocked now).

If I missed other places than scan to adapt and its not to complicated to explain I am always willing to do my best to help ...

I am running HAS Docker with HACS, SSH and Visual Studio Code integration

------ Original Message ------ From "Maikel Punie" @.> To "Cereal2nd/velbus-aio" @.> Cc "sidlgor" @.>; "Author" @.> Date 06/04/2024 09:58:21 Subject Re: [Cereal2nd/velbus-aio] Sequential scan (PR #95)

i looked at it, but this is still a lot of work todo.

there are other priorities to be done before i can start working on this.

— Reply to this email directly, view it on GitHub https://github.com/Cereal2nd/velbus-aio/pull/95#issuecomment-2041009151,

or unsubscribe < https://github.com/notifications/unsubscribe-auth/ACBL57X5PGDZNQ2R4ZKCAULY36TJ3AVCNFSM6AAAAABFRQSTFWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANBRGAYDSMJVGE>.

You are receiving this because you authored the thread.Message ID: @.***>

— Reply to this email directly, view it on GitHub https://github.com/Cereal2nd/velbus-aio/pull/95#issuecomment-2041297320, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA4LF4KCIAT2BRG5YBZ6FL3Y4C57RAVCNFSM6AAAAABFRQSTFWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANBRGI4TOMZSGA . You are receiving this because you commented.Message ID: @.***>

sidlgor commented 5 months ago

Hi

At first: you where absolutely right, there was (is) still a lot of work to do ... It took me some time to create the development environment, but it was worth the effort being able to debug at easy with VS-code. Also the example load_modules helped a lot!

Meanwhile I a have a running, stable version which scans the modules sequential with a smooth load on the canBus and reliable loading, up to the higher module addresses (sea images below) I checked in the code in my forked project and I plan to do now some tests integrated with HASS (modified modules: controller, handler, module, const)

Still I have some questions about the code

Keep you informed about further tests ...

Greets Luc

------ Original Message ------ From "Maikel Punie" @.> To "Cereal2nd/velbus-aio" @.> Cc "sidlgor" @.>; "Author" @.> Date 06/04/2024 09:58:21 Subject Re: [Cereal2nd/velbus-aio] Sequential scan (PR #95)

i looked at it, but this is still a lot of work todo.

there are other priorities to be done before i can start working on this.

— Reply to this email directly, view it on GitHub https://github.com/Cereal2nd/velbus-aio/pull/95#issuecomment-2041009151, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACBL57X5PGDZNQ2R4ZKCAULY36TJ3AVCNFSM6AAAAABFRQSTFWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANBRGAYDSMJVGE. You are receiving this because you authored the thread.Message ID: @.***>

Cereal2nd commented 5 months ago

i'll look at this next week.

can you merge in master? so it has all the changes from master also? Can you also run pre-commit? it seems this part of the CI is failing This makes it simpler to debug, and will resolve all the conflict.

I am a bit confused about the usage of the cache: it seems names are always loaded while they are also available in the cache. Is this intentional?

This was solved in master, where the whole cache mechanisme is re-written

The function PacketHandler._handle_module_type is called from some callback mechanism with an (None) message . Where is this used for? I don't seem to need this callback!

I need to check this, as this should indeed not happen

When the cache is reloaded a lot of consecutive memory loads are requested. They use the message code to get a single byte (0xFD) while the message code "read message block" (0xC9) can load four bytes at once. Use of the block read would significantly reduce the bus load (now 8 messages for a single byte). Is it worth to optimize? Maybe not all module support block read?

Indeed not all modules support this, but this is on my todolist to extend the protocol.json with this info.

Cereal2nd commented 5 months ago

i have been playing with it, but ihave cases where not all modules are correctly loaded (i have 4 test systems where i run all code on befor i push.)

so somethign is not ok yet, but the basis looks ok.

will investigate the coming week.

sidlgor commented 5 months ago

👌

I noticed in the module code that 'is_loaded' is set on reception of last channel name. Because I did not know that, I waited for a 'reasonable time' before continuing with next module Beter would be to enlarge significantly SCAN_MODULEINFO_COMPLETION_TIME and add an extra test

Have a nice weekend ...

 async def scan(self) -> None:
     self._log.info("Start module scan")
     while self._modulescan_address < 254:
         address = 0
         module = None
         with self._scanLock:
             self._modulescan_address = self._modulescan_address + 1
             address = self._modulescan_address
             module = self._velbus.get_module(address)
         self._log.info(f"Starting scan {address} Module {module}")
         if module is None:
             try:
                 self._typeResponseReceived.clear()
                 await self._velbus.sendTypeRequestMessage(address)
                 await

asyncio.wait_for(self._typeResponseReceived.wait(), SCAN_MODULETYPE_TIMEOUT / 1000.) with self._scanLock: module = self._velbus.get_module(address) except asyncio.TimeoutError: self._log.info(f"Scan module {address} failed: not present or unavailable") if not module is None: if not module.is_loaded(): try: self._log.debug(f"Module {address} detected: start loading") await asyncio.wait_for(module.load(from_cache=True), SCAN_MODULEINFO_TIMEOUT / 1000.) self._scan_delay_msec = SCAN_MODULEINFO_COMPLETION_TIME while self._scan_delay_msec > 10 and not module.is_loaded(): self._scan_delay_msec = self._scan_delay_msec - 10

self._log.debug(f"\t... waiting

{self._scan_delay_msec}") await asyncio.sleep(0.01) self._log.info(f"Scan module {address} completed") except asyncio.TimeoutError: self._log.error(f"Module {address} did not respond to info requests after succesfull type request") self._scan_complete = True self._log.info("Module scan completed")

------ Original Message ------ From "Maikel Punie" @.> To "Cereal2nd/velbus-aio" @.> Cc "sidlgor" @.>; "Author" @.> Date 13/04/2024 08:27:08 Subject Re: [Cereal2nd/velbus-aio] Sequential scan (PR #95)

i have been playing with it, but ihave cases where not all modules are correctly loaded (i have 4 test systems where i run all code on befor i push.)

so somethign is not ok yet, but the basis looks ok.

will investigate the coming week.

— Reply to this email directly, view it on GitHub https://github.com/Cereal2nd/velbus-aio/pull/95#issuecomment-2053523456, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACBL57T524OPKUREO3PKFHLY5DF3ZAVCNFSM6AAAAABFRQSTFWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANJTGUZDGNBVGY. You are receiving this because you authored the thread.Message ID: @.***>

Cereal2nd commented 5 months ago

I'm working on this branch:

https://github.com/Cereal2nd/velbus-aio/tree/95

there is still work to be done, not on all test systems its loaded completely

sidlgor commented 4 months ago

Any idea what's going wrong? Works fine with my configuration ... but I don't have all module types!Just latest optimisation did not work as expected because module.isloaded seemed to be not updated ... I did not look further because you were actively on that part...Always prepared to help ...Op 3 mei 2024 12:39 schreef Maikel Punie @.***>: I'm working on this branch: https://github.com/Cereal2nd/velbus-aio/tree/95 there is still work to be done, not on all test systems its loaded completely

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you authored the thread.Message ID: @.***>

sidlgor commented 3 months ago

Hi, please can you put me back in sync with asynchronous scan issue (#95) You where still testing some issues on your test systems ... Thx

sidlgor commented 3 months ago

Hi, I guess you are busy with other more urgent things than the asynchronous scan issue ... Just one (simple) question: until now I have been using load_modules() from your examples to do all my testing independent of has. This was (is) great, but now what is the best way to move the code to has? Add custom integration? Install Velbus has integration and overwrite files? ... Would be a great help ... thx

fyi: I improved async scan code to use the cache to determine the active modules. This makes for a fast and reliable start with minimal stress on canBus. A full (slow) scan is now done only when cache is cleared.

Cereal2nd commented 3 months ago

can you merge in master?

then i'll have a look at it

sidlgor commented 3 months ago

I can't merge directly in master because my fork (sidlgor) is to many commits behind. Anyway, the code is not really the problem (load_modules() works fine as a test, inclusif using cache for active module identification). The problem is just knowing how to load the code (fork) in home assistant. I think most efficient is that I can fully load and test the code in home assistant befor moving to the master. Niobos is helping me on that, but it would be great if youcould assist on that!

A side from Niobos-path I guess it must also be possible to copy the official home assastant repo under custom_components (= first step Niobos) and edit the manifest.json file to refer the velbus-aio package ("requirements": ["velbus-aio==x.x.x"]). The package can be build and installed from the fork with PIP INSTALL.

It would be great to put us on the right track with the HAS integration!!! Afterwords I will be happy to merge my code in the current master so that on the long term we can eliminate my fork. In parallel, and if you can find sufficient time, you could run my fork against your test systems. This would double validate the code (I have a 77 modules installation which loads perfect, but I don't have all types and I also use velserv which also introduces some timing differences in comparison with usb)

------ Original Message ------ From "Maikel Punie" @.> To "Cereal2nd/velbus-aio" @.> Cc "sidlgor" @.>; "Author" @.> Date 25/06/2024 14:12:09 Subject Re: [Cereal2nd/velbus-aio] Sequential scan (PR #95)

can you merge in master?

then i'll have a look at it

— Reply to this email directly, view it on GitHub https://github.com/Cereal2nd/velbus-aio/pull/95#issuecomment-2188773412, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACBL57XM5UFSIMYJ5RNC5Z3ZJFNBTAVCNFSM6AAAAABFRQSTFWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCOBYG43TGNBRGI. You are receiving this because you authored the thread.Message ID: @.***>

Cereal2nd commented 3 months ago

you will have to sync your fork, thats the first step i almost never test code inside hass, i always work with the example scripts

Cereal2nd commented 3 months ago

when testing a bit more i found this one: ERROR:velbus-module:channel 96 does not exist for module @ address <None type:82 address:20 loaded:True loading:False channels:

sidlgor commented 3 months ago

Ok, I will try yo sync my fork ... I also tested, like you proposed, with the example scripts. But finally, when everything works with the scripts, it must be tested in hass.

------ Original Message ------ From "Maikel Punie" @.> To "Cereal2nd/velbus-aio" @.> Cc "sidlgor" @.>; "Author" @.> Date 25/06/2024 18:24:26 Subject Re: [Cereal2nd/velbus-aio] Sequential scan (PR #95)

you will have to sync your fork, thats the first step i almost never test code inside hass, i always work with the example scripts

— Reply to this email directly, view it on GitHub https://github.com/Cereal2nd/velbus-aio/pull/95#issuecomment-2189400881, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACBL57UZM77W6TAZSAJHZX3ZJGKTVAVCNFSM6AAAAABFRQSTFWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCOBZGQYDAOBYGE. You are receiving this because you authored the thread.Message ID: @.***>

Cereal2nd commented 3 months ago

once the code is up to date and merged we will make a pre-release, this will make it easier to test this in hass without the need to enable a full dev enviroment of hass.

sidlgor commented 3 months ago

For all clearity: I don't need, want a full dev environment of hass! I just want my code (fork) to be loaded. Cannot I just make a PIP INSTALL of my code to just make it run and verify if everything is ok?

As mentioned before I cannot just sync my fork because there are to many commits to discard. I don't mind doing the work again later but desperately need a running version for my home installation. I am already blocked now for several months!

About: when testing a bit more i found this one ERROR:velbus-module:channel 96 does not exist for module @ address <None type:82 address:20 loaded:True loading:False channels:

If I am right the type 82 is a VBMELO. I have that module too, set on addresses 20,21. I don't get that error on my system. Did you run the code from my fork? Do you get the error always? Did you use the cache with usecache.yes?

------ Original Message ------ From "Maikel Punie" @.> To "Cereal2nd/velbus-aio" @.> Cc "sidlgor" @.>; "Author" @.> Date 26/06/2024 13:12:26 Subject Re: [Cereal2nd/velbus-aio] Sequential scan (PR #95)

once the code is up to date and merged we will make a pre-release, this will make it easier to test this in hass without the need to enable a full dev enviroment of hass.

— Reply to this email directly, view it on GitHub https://github.com/Cereal2nd/velbus-aio/pull/95#issuecomment-2191434594, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACBL57VM7NSUPPH5F7GQAPLZJKOZVAVCNFSM6AAAAABFRQSTFWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCOJRGQZTINJZGQ. You are receiving this because you authored the thread.Message ID: @.***>

Cereal2nd commented 3 months ago

without a merge to master i can not create a pre-release, so i can not do anything to help you without setting up the whole hass dev enviroment.

i tested it on your branch and it seems good, but to be able to easely test this in hass we need it in master

sidlgor commented 3 months ago

Ok, no problem, I can understand that ... I discarded the commits on my fork, and as I expected, lost about every edit I made. Don't worry, I have a safe copy of the new code 😉

Because I really miss git experience (remember I am a Microsoft TFS guy) here some questions to try to do it right this time

I will wait for your response and try to do everything the day after ... Thx again for making some time in your busy agenda, much appreciated!

------ Original Message ------ From "Maikel Punie" @.> To "Cereal2nd/velbus-aio" @.> Cc "sidlgor" @.>; "Author" @.> Date 26/06/2024 18:01:36 Subject Re: [Cereal2nd/velbus-aio] Sequential scan (PR #95)

without a merge to master i can not create a pre-release, so i can not do anything to help you without setting up the whole hass dev enviroment.

i tested it on your branch and it seems good, but to be able to easely test this in hass we need it in master

— Reply to this email directly, view it on GitHub https://github.com/Cereal2nd/velbus-aio/pull/95#issuecomment-2192066218, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACBL57W3JP2FCXCX3BQ7GNTZJLQWBAVCNFSM6AAAAABFRQSTFWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCOJSGA3DMMRRHA. You are receiving this because you authored the thread.Message ID: @.***>

sidlgor commented 3 months ago

Fork sidlgor has been synced and afterwards updated to newest async scan code. I don't know next step for moving to master ... don't know how to request merging, pulling ?? Going to bed now ...

------ Original Message ------ From "Maikel Punie" @.> To "Cereal2nd/velbus-aio" @.> Cc "sidlgor" @.>; "Author" @.> Date 26/06/2024 18:01:36 Subject Re: [Cereal2nd/velbus-aio] Sequential scan (PR #95)

without a merge to master i can not create a pre-release, so i can not do anything to help you without setting up the whole hass dev enviroment.

i tested it on your branch and it seems good, but to be able to easely test this in hass we need it in master

— Reply to this email directly, view it on GitHub https://github.com/Cereal2nd/velbus-aio/pull/95#issuecomment-2192066218, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACBL57W3JP2FCXCX3BQ7GNTZJLQWBAVCNFSM6AAAAABFRQSTFWVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCOJSGA3DMMRRHA. You are receiving this because you authored the thread.Message ID: @.***>

Cereal2nd commented 3 months ago

Create a new merge request.

We can test on that basis and then merge it to master