Differential D14827

Improve regex for temperatures
ClosedPublic
Actions

Authored by laysrodrigues on Aug 14 2018, 11:07 AM.

Details

Reviewers
rizzitello
patrickelectric
tcanabrava
Commits
R232:3b0ee4fcfa76: Improve regex for temperatures
R232:669c6440f7b3: Improve regex for temperatures
R232:26e43a1ed45c: Add new test for marlin creality flavor
Summary

Signed-off-by: Lays Rodrigues <lays.rodrigues@kde.org>

Add new test for marlin creality flavor

Signed-off-by: Lays Rodrigues <lays.rodrigues@kde.org>

Diff Detail

Repository
R232 AtCore
Branch
improve_regex_for_temps
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 1824
Build 1842: arc lint + arc unit
laysrodrigues requested review of this revision.Aug 14 2018, 11:07 AM
laysrodrigues created this revision.
rizzitello requested changes to this revision.EditedAug 14 2018, 11:11 AM

I guess just appending this to the previous revision was to much ? (D14792)
The decodeTemp function is fine as i wrote and tested it last night.

Please use a real return from the printer for your test
Please squash this into one commit that contains both the change and the new test.

This revision now requires changes to proceed.Aug 14 2018, 11:11 AM
rizzitello added a comment.Aug 14 2018, 11:16 AM

Just noticed that the Target Regex does not capture after decimal point . Fix inline :D

src/core/temperature.cpp
99

T:[^\\/]*\\/(?<extruderTarget>(\\d+.\\d+))

to capture after the decimal place

115
B:[^\\/]*\\/(?<bedTarget>(\\d+.\\d+))

to capture after the decimal place

rizzitello added inline comments.Aug 14 2018, 11:24 AM
src/core/temperature.cpp
99

That is wrong use (adjusted for the T or B of course)

B:[^\\/]*\\/(?<bedTarget>((\\d+.\\d+|\\d+)))
This will capture with the decimal (perfered) or with out if the decmial is not present

https://regex101.com/r/zNvUBm/2

115

See note on other

laysrodrigues added a comment.Aug 14 2018, 11:29 AM
In D14827#308557, @rizzitello wrote:

I guess just appending this to the previous revision was to much ? (D14792)
The decodeTemp function is fine as i wrote and tested it last night.

Please use a real return from the printer for your test
Please squash this into one commit that contains both the change and the new test.

Tomaz abandoned that because I did the revision, so he asked to me to do a new one with my phabricator authentication.

src/core/temperature.cpp
99

it isnt because all the tests pass, and I am currently printing without any issue.

115

No need for that. As we only show the integer value on the widgets.

rizzitello added inline comments.Aug 14 2018, 11:35 AM
src/core/temperature.cpp
99

Do it correctly or don't do it at all.. if you don't want your widgets to show decimal places and only int values thats up to you but don't restrict those who want to see the real numbers.

115

Do it correctly or don't do it at all.. if you don't want your widgets to show decimal places and only int values thats up to you but don't restrict those who want to see the real numbers.

laysrodrigues updated this revision to Diff 39690.Aug 14 2018, 1:02 PM
  • Sith suggestions
rizzitello accepted this revision.Aug 14 2018, 1:08 PM

Please squash commits so the test comes with the newer reg ex.

This revision is now accepted and ready to land.Aug 14 2018, 1:08 PM
laysrodrigues closed this revision.Aug 14 2018, 1:22 PM

Revision Contents
Changeset List

Diff 39690

View Options

src/core/temperature.cpp

Loading...
View Options

unittests/temperaturetests.h

Loading...
View Options

unittests/temperaturetests.cpp

Loading...