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

Change all require() paths to relative #40

Open
wants to merge 1 commit into
base: sdk-v3
Choose a base branch
from

Conversation

roelofoomen
Copy link

As described in the developer guide.

@roelofoomen roelofoomen changed the base branch from main to sdk-v3 June 28, 2023 04:09
@roelofoomen
Copy link
Author

With these changes the app seems to run fine on my Homey Pro 2023.

@Joolee
Copy link
Owner

Joolee commented Jun 28, 2023

The app has been updated far enough now that it will start without immediately running into a problem but I still had the app crashing on my 2016 Homey after running for a while.
Also haven't tested actually doing anything with the app yet like adding or removing devices, receiving events and changing settings. I still expect there to be a lot of crashes testing that, that's why I haven't released anything yet.

About your patch; seems to be a 2023 Homey thing, needing relative paths. I can't test that as I don't have the new model but I'll assume it's necessary :) Unfortunately, your autoformatting plugin had made thousands of changes throughout the codebase. Can you change the patch to only include the actually functional code chances?

@roelofoomen
Copy link
Author

Oh, indeed, I didn't notice all the formatting changes, I'm sorry. I'll fix this and resubmit.

I'm running the app currently with one Wemos D1 Mini connected and it seems stable and working for this single case. This was a device that was added already using the previous version of the app on my Homey Pro 2019 (I restored a backup to my new Homey), so I haven't tested adding devices and ports etc.

@roelofoomen
Copy link
Author

I fixed the PR. All formatting changes have been removed.

@roelofoomen
Copy link
Author

I've been using this version since June now, with one ESP device (Wemos D1 Mini) with 5 GPIO Pulse Outputs, without noticing any issues.

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.

3 participants