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

Update CGDK2_json.h #602

Merged
merged 2 commits into from
Jan 20, 2025
Merged

Update CGDK2_json.h #602

merged 2 commits into from
Jan 20, 2025

Conversation

delboy711
Copy link
Contributor

Correct 'name' for PVVX firmware

Description:

Alternative PVVX firmware for CDGK2 assigns a name for the device in the format CGD_XXXXXX as in this example message.
{"servicedatauuid": "181a", "servicedata": "fab582342d583e080c14840b5dc00e", "name": "CGD_82B5FA", "id": "58:2D:34:82:B5:FA", "rssi": -38}

The file CGDK2_json.h is searching for a name containing the text "CDGK" so decoding fails.
Searching for a shorter string of "CDG" fixes the issue.
I have tested this patch with PVVX firmware rev48.

Checklist:

  • The pull request is done against the latest development branch
  • Only one feature/fix was added per PR and the code change compiles without warnings
  • I accept the DCO.

Correct 'name' for PVVX firmware
@DigiH
Copy link
Contributor

DigiH commented Jan 18, 2025

Thanks @delboy711

This seems to be either a new PVVX firmware version or the firmware denoting a different hardware revision of the CGDK2, as previous/different version had/have the CGDK2_DDEEFF naming.

Could you add an additional test case with your above undecoded data, so that this is ducumented for future regression testing, possibly below the current existing one

Test case
https://github.com/theengs/decoder/blob/development/tests/BLE/test_ble.cpp#L1012

Decoder ID
https://github.com/theengs/decoder/blob/development/tests/BLE/test_ble.cpp#L1106

Expected result
https://github.com/theengs/decoder/blob/development/tests/BLE/test_ble.cpp#L260

@delboy711
Copy link
Contributor Author

Patch for test_ble.cpp is at delboy711@1ae3f3a

Hope its OK.

@DigiH
Copy link
Contributor

DigiH commented Jan 18, 2025

Hope its OK.

Thanks, the Decoder ID needs to be the same as the one above, i.e.

TheengsDecoder::BLE_ID_NUM::CGDK2_PVVX,

as this is how the whole decoder has its ID defined, regardless of the model condition change.

And also the model_id will remain

… \"model_id\":\"CGDK2_PVVX\" …

in the expected result.

With these changes merged into this PR the checks should all run fine.

@DigiH
Copy link
Contributor

DigiH commented Jan 20, 2025

@delboy711

Don't you want to merge your above linked patch with the test case with my suggested changes into this PR? Then we could merge it.

@delboy711
Copy link
Contributor Author

Sorry for the delay. I had to learn how PRs work in Gitgub. Hopefully all OK now.

@DigiH
Copy link
Contributor

DigiH commented Jan 20, 2025

Looking all fine.

Thanks again.

@DigiH DigiH merged commit 910f4e0 into theengs:development Jan 20, 2025
7 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