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

fix 8139 netbios in AP mode #8140

Closed
wants to merge 3 commits into from
Closed

fix 8139 netbios in AP mode #8140

wants to merge 3 commits into from

Conversation

pabloandresm
Copy link
Contributor

fix for NetBIOS only working if WIFI in STA mode and not AP mode

#8139

@earlephilhower
Copy link
Collaborator

@pabloandresm, I don't think any maintainers use the NetBIOS lib. Have you personally tried this change with AP mode?

@d-a-v d-a-v added this to the 3.0.1 milestone Jun 24, 2021
@d-a-v d-a-v added the waiting for feedback Waiting on additional info. If it's not received, the issue may be closed. label Jun 24, 2021
@earlephilhower earlephilhower modified the milestones: 3.0.1, 3.1 Jun 26, 2021
@d-a-v
Copy link
Collaborator

d-a-v commented Jun 15, 2022

pushing to v4 per still in discussion

edit:
Approving per feedback.
Changes look good, maybe not fully aware of all network states (ap+sta?), also not ethernet aware.
Netbios users will report back in case of further issues.

@d-a-v d-a-v modified the milestones: 3.1, 4.0.0 Jun 15, 2022
@d-a-v d-a-v removed the waiting for feedback Waiting on additional info. If it's not received, the issue may be closed. label Jun 18, 2022
@mcspr
Copy link
Collaborator

mcspr commented Jun 19, 2022

pushing to v4 per still in discussion

edit: Approving per feedback. Changes look good, maybe not fully aware of all network states (ap+sta?), also not ethernet aware. Netbios users will report back in case of further issues.

why not upcb->local_ip though? we do write a low level code interacting with the lwip here
why mix in WiFi class at all?

why not the previously suggested ip_current_dest_addr()
or, in case that is broadcast for whatever reason, ip_current_netif() and get local ip from there?

@d-a-v
Copy link
Collaborator

d-a-v commented Jun 27, 2022

@mcspr As no-one can make and test a more robust change, I suggest, as part of the current freezing process, to merge this bugfix before of after adding this comment

// Note from maintainers: for further improvements, see
// see https://github.com/esp8266/Arduino/pull/8140#discussion_r654549861
// and https://github.com/esp8266/Arduino/pull/8140#issuecomment-1159587969
if (WiFi.getMode()==WIFI_STA)
...

@mcspr
Copy link
Collaborator

mcspr commented Jun 27, 2022

@d-a-v let me check the change and get back to you... it also seems to generate a malformed packet

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