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

openthread-br: bump source to 2024-11-20 and add missing dependencies #25412

Merged
merged 1 commit into from
Nov 25, 2024

Conversation

itay-sho
Copy link
Contributor

@itay-sho itay-sho commented Nov 19, 2024

Maintainer: @stintel
Compile tested: armsr/armv8/generic, and banana pi r4. both aarch64
Run tested: armsr/armv8/generic, and banana pi r4. both aarch64
I tested it until I saw that the service is up (and not crashing) as it was before

Description:
bumped ot-br-posix that openthread-br is based on to latest code.
the major changes required some more compilation flags, and in order to avoid the ot-br agent had to add some dependencies as weell.

dependencies:

  1. kmod-tun: otherwise the otbr fails to load and crashed immidiedtly

compilation flags:

  1. OTBR_NAT64, OT_NAT64_BORDER_ROUTING: a temporary workaround to a compilation problem with openwrt. there is an issue I've opened about it here: Failing to compile to openwrt openthread/ot-br-posix#2606
  2. OT_TARGET_OPENWRT: wasn't assumed from OTBR_OPENWRT in banana pi r4 (but was in the other target). decided to add it to avoid problem

@stintel
Copy link
Member

stintel commented Nov 19, 2024

I'll have to investigate this properly. I cannot believe avahi is needed and I absolutely do not want to add that dependency. Unfortunately I am renovating my house and this will have to wait. To anyone with merge privileges on this repository: do not merge this until further notice.

@itay-sho
Copy link
Contributor Author

I'll have to investigate this properly. I cannot believe avahi is needed and I absolutely do not want to add that dependency. Unfortunately I am renovating my house and this will have to wait. To anyone with merge privileges on this repository: do not merge this until further notice.

There is a possibility that adding OT_TARGET_OPENWRT removed avahi client as a mandatory dependency for this. Both were required only on my bpi4 target (and not the other qemu arm target). it did compile a little different after adding it.

Would you like me to check?

@stintel
Copy link
Member

stintel commented Nov 19, 2024

I'll have to investigate this properly. I cannot believe avahi is needed and I absolutely do not want to add that dependency. Unfortunately I am renovating my house and this will have to wait. To anyone with merge privileges on this repository: do not merge this until further notice.

There is a possibility that adding OT_TARGET_OPENWRT removed avahi client as a mandatory dependency for this. Both were required only on my bpi4 target (and not the other qemu arm target). it did compile a little different after adding it.

Would you like me to check?

Please do. I really don't want avahi and dbus as dependency of this package.

@itay-sho itay-sho force-pushed the bump-ot-br-posix-version branch 2 times, most recently from bbdde7f to 8c63128 Compare November 20, 2024 18:30
@itay-sho
Copy link
Contributor Author

After removing libavahi and enabling OT_TARGET_OPENWRT, the compilation was successful.
The commit was recreated without libavahi-client as a dependency.
Additionally, the ot-br-posix version was bumped to include the latest commits made since I began working on this.

wdyt?

@itay-sho itay-sho changed the title openthread-br: bump source to 2024-11-07 and add missing dependencies openthread-br: bump source to 2024-11-20 and add missing dependencies Nov 20, 2024
@neheb
Copy link
Contributor

neheb commented Nov 20, 2024

Meh I bought 2 USB sticks to test with this. Never got around to it...

@stintel
Copy link
Member

stintel commented Nov 21, 2024

I will try to test this over the weekend. I am using 3 nanoleaf essential bulbs, an EVE energy, and an Aqara U200 smart lock on my OpenWrt Thread network. If these keep working I'll accept the PR.

@stintel
Copy link
Member

stintel commented Nov 21, 2024

Could you fix the test formalities failure though?

: Signed-off-by is missing or doesn't match author (should be 'Signed-off-by: Itay Shoshani itai.sho@gmail.com')

net/openthread-br/Makefile Outdated Show resolved Hide resolved
net/openthread-br/Makefile Outdated Show resolved Hide resolved
@stintel
Copy link
Member

stintel commented Nov 21, 2024

And maybe add some of the reasoning you mentioned in the comments for changes in the commit message.

@itay-sho itay-sho force-pushed the bump-ot-br-posix-version branch from 8c63128 to 5b71b34 Compare November 21, 2024 14:01
…ndencies

Bumping ot-br-posix that openthread-br is based on to latest code.
Due to some major changes required some more compilation flags, and in order to avoid the ot-br agent had to add some dependencies as well

Dependecies:
1. kmod-tun: otherwise the otbr fails to load and crashed immidiedtly

Compilation flags:
1. OTBR_NAT64, OT_NAT64_BORDER_ROUTING set to OFF: a temporary workaround to a compilation problem with openwrt, could be reverted once the issue here is fixed: openthread/ot-br-posix#2606
2. OT_TARGET_OPENWRT: wasn't assumed from OTBR_OPENWRT in some targets

Signed-off-by: Itay Shoshani <itai.sho@gmail.com>
@itay-sho itay-sho force-pushed the bump-ot-br-posix-version branch from 5b71b34 to ecb5fa7 Compare November 21, 2024 14:04
@itay-sho
Copy link
Contributor Author

Worked on the changes you talked about
Can you take a look?

@stintel
Copy link
Member

stintel commented Nov 25, 2024

Had guests over the weekend, but just tested this and can't spot any immediate regressions.

@stintel stintel merged commit c72af1e into openwrt:master Nov 25, 2024
15 checks passed
@itay-sho itay-sho deleted the bump-ot-br-posix-version branch November 25, 2024 20:43
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