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

Split code. #77

Closed
Closed

Conversation

FabienD74
Copy link
Contributor

Original version

@vingerha
Copy link
Owner

You are changing things ...do let me know when this is clean from device-tracker related things and ready for review, no time to check over and over again :)

@FabienD74
Copy link
Contributor Author

I messed up will all my backup folders and repositories.
The commit named "Add basic event/listner" contains too much. Fixed by the third commit, but it's not clean...
Sorry.

I restart everything if you want...

@vingerha
Copy link
Owner

vingerha commented May 22, 2024

Deploying PR is a not as straightforward ... I know.... just let me know when done
Another comment... the (new) local stop sensor list will eventually pose issues as it will grow out of acceptable size (attributes also have a limit) ...esp. if you are in a big town and/or big radius....

@FabienD74
Copy link
Contributor Author

i have no quick easy solution.
If u think there is a risk, then may be we should revert everyting back... and delete this PR.

@FabienD74
Copy link
Contributor Author

I will continue on my fork.... I still need to find out the "destination" of the route. Otherwise this whole development is useless.
Regards

@vingerha
Copy link
Owner

Yep...do continue and you can run all you build for yourselves of course. I can also have a look at the fork when you think you are done..merging with my main needs a reasonably OK basis (perfect does not exist :) ). For the local list, yes...this is going to be a possible issue...this is the 'warning' that will follow in such cases, not sure if it truncates

.... exceed maximum size of 16384 bytes. This can cause database performance issues; Attributes will not be stored
` `

@vingerha
Copy link
Owner

I will continue on my fork.... I still need to find out the "destination" of the route. Otherwise this whole development is useless. Regards

I can only say good-luck but I have to admit that I personally donot dee that happening

@FabienD74
Copy link
Contributor Author

Thanks for your research.... => 16Kb ... that should be enough for my needs...
It's already a huge timetable if I remove some info like route_name, coords,...

What a week!!!
First dev in python 👍, first tme with github 👍, and first "custom component" ! 👍

That's too much in parallel ;-) ;-) ;-)
Sorry

@vingerha
Copy link
Owner

vingerha commented May 22, 2024

Hence the start small, there is a lot of thinking and issue-resolving in this code...it is not perfect at all but soo many things went peashaped due to unexpected issues with the source-data, database, HA-use, etc. So, if you want to put that sensor in a separate file then do that first, then test, apply further settings. The list is still an issue, it may be small enough for you but not for others and I do not want to start a repair-cycle, so if it breaks, I will just remove all of it and go back to 'today'. With me, simple env. the sensor already reaches 10k+ for 6 stops and with a low (village-alike) frequency

@vingerha
Copy link
Owner

Further small test.. I added a zone in amsterdam with many stop sbut there is no stop and no list, als the one that showed above 10k before no longer pops-up with data. I will revert this as it is not stable... thorough testing is required first.
I will add a DEV branch so you can sync with that

image

@vingerha
Copy link
Owner

vingerha commented May 22, 2024

New branch dev created on the 'main' based on 'main' before applying your previous PR.
I can create releases either from main or dev so this at least keeps 'main' as clean as can be ... and donot get me wrong, I do have to return quite often on my code :)
Still, I propose you first create a working version on your fork, test this for zone and person and for multiple sources .... and when ready I will test it out as well

@FabienD74
Copy link
Contributor Author

Perfect!

@vingerha
Copy link
Owner

btw... to avoid making this a semi-chat post ...I am on disocrd with the same name, easier and less polluting for others :)

@vingerha
Copy link
Owner

Closing as this has mostly been used for chatty stuff and the PR is not complete

@vingerha vingerha closed this Jun 16, 2024
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

2 participants