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

SML: FastExit for binary SML parsing #21497

Merged
merged 4 commits into from
May 30, 2024

Conversation

rPraml
Copy link
Contributor

@rPraml rPraml commented May 25, 2024

Description:

This improves performance for the SML parser for type 's' (SML binary smart message coding)
We assume, that an SML message always starts with 0x77 / SML_SYNC and start decoding only, if the buffer starts with the sync byte.
If there is a rare case, that an SML message does not start with 0x77, this improvement can be disabled with flag 32

Details:

I noticed a high load (~170) and slow FPS on the connected display (240x240 ST7789)

With this PR I tried to improve the SML parsing for binary SML data, which seems to have some room for improvement.

As far as I understand the binary parsing, there is a buffer, where each byte is added at the end of the buffer and then the buffer is shifted one byte to the left.
After each byte, SML_Decode is called, which parses the the descriptor each time and checks if one line matches with the buffer.
(@gemu2015 is this correct?)

In my case I have 3 Logarex LK13BE606739 meters, where each transmits ~660 bytes each second.
My descriptor has ~ 60 lines, which means that SML_Decode is called ~2000 times/sec. and it will parse and check each line in the descriptor.

As far as I see, all SML messages in binary mode will start with 0x77, so I thought, it could be a good idea to implement a fast exit, and start decoding only, when the buffer starts with the sync byte, which makes the decoding a lot of faster.
(0x77 occurs 23 times in my test sample of 660 bytes)

To test the improvement, I bound this to flag 32, but I want to discuss, if this could be the default. (is 0x77 really a sync byte?)

This change reduces the load from 170 to 100 and gives some extra frames on the TFT

Roland

Related issue (if applicable): fixes #

Checklist:

  • The pull request is done against the latest development branch
  • Only relevant files were touched
  • Only one feature/fix was added per PR and the code change compiles without warnings
  • The code change is tested and works with Tasmota core ESP8266 V.2.7.6
  • The code change is tested and works with Tasmota core ESP32 V.3.0.0
  • I accept the CLA.

NOTE: The code change must pass CI tests. Your PR cannot be merged unless tests pass

@gemu2015
Copy link
Contributor

that seems to be a good improvement.
i would suggest to make this the default und invert flag & 32 as i also assume that 0x77 is always present.

@rPraml
Copy link
Contributor Author

rPraml commented May 26, 2024

I've modified the PR a bit. I still have to test this and provide a PR for documentation update...

tasmota/tasmota_xsns_sensor/xsns_53_sml.ino Outdated Show resolved Hide resolved
@rPraml rPraml changed the title Please review: SML: FastExit for binary SML parsing SML: FastExit for binary SML parsing May 28, 2024
rPraml added a commit to rPraml/docs that referenced this pull request May 28, 2024
@rPraml
Copy link
Contributor Author

rPraml commented May 28, 2024

This PR is tested on an ESP32-S3 and works IMHO

struct METER_DESC *md = &meter_desc[index];
// start of serial source buffer
cp = md->sbuff;
if (md->flag & 32 && md->type == 's' && *cp != SML_SYNC) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't the code be easier to understand if a constant with a descriptive name were used for the numerical value 32? And since a bit is obviously being tested here, the use of 0x20 is a good idea.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is this the same comment as #21497 (review) ?
This is already fixed.

@arendst arendst merged commit 4d9e919 into arendst:development May 30, 2024
59 checks passed
blakadder pushed a commit to tasmota/docs that referenced this pull request Jun 17, 2024
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