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

Ensure message delivery for NASA packets #93

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

atanasenko
Copy link
Contributor

@atanasenko atanasenko commented Feb 11, 2024

This is a draft of my playground which I was testing with my NASA 3 indoor to 1 outdoor units and trying to fix the writes which were either ignored or also caused some incoming packets to get mangled (sometimes including acks for a successful message). I'd like to open a discussion and am open for any suggestions.

I'm running this code locally and so far I haven't had a single missed packet for quite a while.

I don't have a way to test this with non-nasa, so I retained the behavior where packets were sent immediately after F8.

I have a suspicion that the preamble bytes that are sent prior to each packet might have something in them for congestion control. But until somebody figures this out, sending packets has to happen during quiet periods.

  • Queue outgoing packets and send them during loop()
  • Only send packets during relatively quiet time
  • Do not send new packets until we receive an ack for the one already sent
  • Retry sending packet if no ack received
  • Make absolutely sure that there is no incoming data available or there is no more data expected to arrive before sending
  • Attempt to recover packets from incoming data if stars aligned in a way that all of the above failed
  • Modify raw data logging a bit for readability

See #57

@atanasenko atanasenko force-pushed the transport branch 6 times, most recently from d8082f4 to e727d98 Compare February 12, 2024 03:35
@lanwin
Copy link
Collaborator

lanwin commented Feb 12, 2024

Thank you for that draft. Can you have a look into the test stuff. That is currently broken. Short reminder. That test stuff is designed to run without esphome.

Regarding the preamble I have found this post https://buildingenergy.cx-associates.com/what-is-rs-485-part-2 . It seems that this is more to make devices sync up on the right boud rate.

@atanasenko
Copy link
Contributor Author

I'll try to find some time to work on the tests this weekend.

@lanwin
Copy link
Collaborator

lanwin commented Feb 21, 2024

Do you had time to revisit this? Also there are merge conflicts.

* Queue outgoing packets and send them during loop()
* Only send packets during relatively quiet time
* Do not send new packets until we receive an ack for the one already sent
* Retry sending packet if no ack received
* Make absolutely sure that there is no incoming data available or there is no more data expected to arrive before sending
* Attempt to recover packets from incoming data if stars aligned in a way that all of the above failed
* Modify logging to be able to easily change log levels during development
* Optimize passing data buffers back and forth to prevent extra copies
@htht2001
Copy link

htht2001 commented Oct 7, 2024

@atanasenko
Is there a merger expected?
Your contribution is essential for for use in air conditioners
Thank you very much

@lanwin
Copy link
Collaborator

lanwin commented Oct 8, 2024

It seems @atanasenko did not have the time anymore to look at it.

It also did not makes sens to fix the merge conflicts. I plan to split Nasa and NonNasa into two different component. Then this could be handled directly within the nasa protocol file.

@htht2001
Copy link

htht2001 commented Oct 8, 2024

Just stating that this branch is used
Commands are handled correctly
By sending to a single air conditioner
and even by sending to several air conditioners simultaneously

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