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 issue #5193: Optimise GPS probe code - one message per family #5244

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

absolut3ego
Copy link

I have upgraded the PROBE_SIMPLE macro. So that it can not need to do everything again when calling the same message. I simply added some variables to save the last command and based on it to execute the next commands, if it matches the latest command I do not need to do the steps in the PROBE_SIMPLE macro again but instead I will get the result and return it.

@CLAassistant
Copy link

CLAassistant commented Nov 4, 2024

CLA assistant check
All committers have signed the CLA.

@fifieldt
Copy link
Contributor

fifieldt commented Nov 4, 2024

Thanks for this! Taking a look :)

@fifieldt
Copy link
Contributor

fifieldt commented Nov 4, 2024

Two things you can do while waiting for review:

@absolut3ego absolut3ego closed this Nov 4, 2024
@absolut3ego absolut3ego reopened this Nov 4, 2024
Copy link
Contributor

@fifieldt fifieldt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this. A great beginning.

Further changes are needed. At the moment this checks whether we got a response to the command, which is a good start.

However, we actually care about the contents. Since the response to the command could be OK, but might correspond to a model number we don't support.

For example, we send the command $PCAS06,11A to look for ATGM336H or ATGM332D. When we see a response of $GPTXT,01,01,02,HW=ATGM336H we know the chip we have is an ATGM336H. If we see a response of $GPTXT,01,01,02,HW=ATGM332D we know it's an ATGM332D. However, we could also see $GNTXT,01,01,01,PCAS inv format23. where it's an invalid command for the chip connected , or it could be $GPTXT,01,01,02,HW=ATGM330B , which we don't support.

So the code should be extended to make the buffer from the first time we run the command available, so we can search it next time.

@fifieldt
Copy link
Contributor

Hi @absolut3ego , did you need any help or pointers with this? I have a lot of GPS to test with, so if you want to make the code so can test it for you :)

if (getACK(RESPONSE, TIMEOUT) == GNSS_RESPONSE_OK) { \
LOG_INFO(DETECTED_MESSAGE, CHIP, #DRIVER); \
return DRIVER; \
#define PROBE_SIMPLE(CHIP, TOWRITE, RESPONSE, DRIVER, TIMEOUT, ...) \
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why don't you implement this as a (lambda-) function? This macro code is copied 10 times into the source and consumes unnecessarily additional space.

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.

4 participants