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

Add General Motors TPMS #3191

Merged
merged 7 commits into from
Feb 13, 2025
Merged

Add General Motors TPMS #3191

merged 7 commits into from
Feb 13, 2025

Conversation

e100
Copy link
Contributor

@e100 e100 commented Feb 8, 2025

This adds a decoder for one of the protocols discussed in #3123

@@ -0,0 +1,161 @@
/** @file
General Motors Aftermarket TPMS
Copy link
Collaborator

Choose a reason for hiding this comment

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

End the first line with a dot.

*/

#include "decoder.h"
#include "r_util.h"
Copy link
Collaborator

Choose a reason for hiding this comment

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

include only decoder.h if possible.

#include <stdio.h>
#include <inttypes.h> // Needed for PRIX64

static uint8_t const preamble_pattern[6] = {0x00, 0x00, 0x00, 0x00, 0x00, 0x00};
Copy link
Collaborator

Choose a reason for hiding this comment

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

move this into the decoder function.

static uint8_t const preamble_pattern[6] = {0x00, 0x00, 0x00, 0x00, 0x00, 0x00};

/**
Data was detected an initially captured using:
Copy link
Collaborator

Choose a reason for hiding this comment

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

first line should be the same as line 2

/**
Data was detected an initially captured using:

rtl_433 -X 'n=name,m=OOK_MC_ZEROBIT,s=120,l=0,r=15600'
Copy link
Collaborator

Choose a reason for hiding this comment

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

four spaces indent for code lines




130 bits
Copy link
Collaborator

Choose a reason for hiding this comment

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

indent

130 bits
AAAAAAAAAAAASSSSDDDDIIIIIIPPTTCCX
0000000000004c90007849176600536d0
Data layout:
Copy link
Collaborator

Choose a reason for hiding this comment

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

no indent for header lines

Bit 5 of status indicates low battery when set to 1
Bits 0,1,8 are set to 0 to indicate learn mode and 1 for operational mode.
The sensors drop to learn mode when detecting a large pressure drop
or when activated with the learning tool
Copy link
Collaborator

Choose a reason for hiding this comment

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

end sentences with a dot

// But I think it might be best to allow the user to
// to add their own offset when consuming the data
float pressure_kpa = (pressure_raw * 2.75);
float pressure_psi = kpa2psi(pressure_kpa);
Copy link
Collaborator

Choose a reason for hiding this comment

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

output native units, don't convert in decoders

int bit0 = (status >> 0) & 1;

// Status bits
int learn_mode = ((bit8 == 0) && (bit1 == 0) && (bit0 == 0)) ? 1 : 0;
Copy link
Collaborator

Choose a reason for hiding this comment

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

don't ? 1 : 0 on a bool

@zuckschwerdt
Copy link
Collaborator

Thanks! Some code style comments :)

@e100
Copy link
Contributor Author

e100 commented Feb 8, 2025

@zuckschwerdt The docs suggest id should be integer but I noticed that all of the TPMS decoders output a hex string for id, should I change my code to be consistent with other TPMS?

@zuckschwerdt
Copy link
Collaborator

A plain int for "id" is preferred. You can use something like "id", "", DATA_FORMAT, "%08x", DATA_INT, id, if you want the hex display in the console to easier match with an id printed on the sensors.

@e100
Copy link
Contributor Author

e100 commented Feb 8, 2025

I believe all of your suggestions have been addressed.

@zuckschwerdt
Copy link
Collaborator

Thanks!
For the decoder doc-comment, indent only code blocks, and those by four spaces. E.g. this

/**
    General Motors Aftermarket TPMS.
Data was detected and initially captured using:
    rtl_433 -X 'n=name,m=OOK_MC_ZEROBIT,s=120,l=0,r=15600'
    130 bits
AAAAAAAAAAAAFFFFDDDDIIIIIIPPTTCCX
0000000000004c90007849176600536d0
Data layout:
   AAAAAAAAAAAAFFFFDDDDIIIIIIPPTTCCX

should be


/**
General Motors Aftermarket TPMS.

Data was detected and initially captured using:
    rtl_433 -X 'n=name,m=OOK_MC_ZEROBIT,s=120,l=0,r=15600'

130 bits:
    AAAAAAAAAAAAFFFFDDDDIIIIIIPPTTCCX
    0000000000004c90007849176600536d0

Data layout:
    AAAAAAAAAAAAFFFFDDDDIIIIIIPPTTCCX

computed_checksum % 256 is usually written computed_checksum & 0xff.

flags, pressure_raw, temperature_raw should just be int.

The model must follow https://triq.org/rtl_433/DATA_FORMAT.html, likely GM-Aftermarket.

Run ./maintainer_update.py to get the readme changes.

@e100
Copy link
Contributor Author

e100 commented Feb 10, 2025

@zuckschwerdt
Thanks for the guidance and patience.

Hopefully I have not overlooked anything this time.
I've also sent a pull request to the test repo with some recordings and json

@zuckschwerdt zuckschwerdt merged commit 474feb5 into merbanan:master Feb 13, 2025
9 checks passed
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.

2 participants