New AxisControl Widget
ClosedPublic

Authored by rizzitello on Sep 5 2018, 4:44 AM.

Diff Detail

Repository
R232 AtCore
Branch
newaxiscontrol
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 2701
Build 2719: arc lint + arc unit
rizzitello requested review of this revision.Sep 5 2018, 4:44 AM
rizzitello created this revision.
rizzitello edited the summary of this revision. (Show Details)Sep 5 2018, 4:45 AM
rizzitello added a project: Atelier: AtCore.
rizzitello added a subscriber: Atelier: AtCore.
laysrodrigues requested changes to this revision.Sep 8 2018, 2:14 PM
laysrodrigues added inline comments.
src/widgets/axiscontrol.cpp
2

same

src/widgets/axiscontrol.h
2

2016-2018

This revision now requires changes to proceed.Sep 8 2018, 2:14 PM
rizzitello updated this revision to Diff 41211.Sep 8 2018, 4:56 PM
  • adjust copyright span
patrickelectric requested changes to this revision.Sep 9 2018, 12:58 AM
patrickelectric added inline comments.
src/widgets/axiscontrol.cpp
21

Alphabetic order

src/widgets/axiscontrol.h
50

Sorry for asking, but why explicit is necessary here ?

51

= default

58

QLatin1Char&

This revision now requires changes to proceed.Sep 9 2018, 12:58 AM
rizzitello updated this revision to Diff 41234.Sep 9 2018, 1:12 AM
rizzitello marked 4 inline comments as done.
  • patrick suggestions
rizzitello marked 2 inline comments as done.Sep 9 2018, 1:13 AM
patrickelectric accepted this revision.Sep 9 2018, 1:28 AM

I think that need the unit showing in the textbox, like mm, cm, m.

rizzitello added a comment.EditedSep 9 2018, 7:21 PM

I think that need the unit showing in the textbox, like mm, cm, m.

not possible since we will not know if the machine is using metric or imperial units. that is why i just used "units"

I think that need the unit showing in the textbox, like mm, cm, m.

not possible since we will not know if the machine is using metric or imperial units. that is why i just used "units"

can this be configured? (the metrics or imperial?)

src/widgets/axiscontrol.h
58

sorry, patrick's wrong here.
QLatin1Char should be passed as value, not ref.
(the sizeof QLatin1Char is smaller than a double)

src/widgets/axiscontrol.h
58

The documentation says that is a class with functions.
http://doc.qt.io/qt-5/qlatin1char.html

struct QLatin1Char
{
public:
    Q_DECL_CONSTEXPR inline explicit QLatin1Char(char c) Q_DECL_NOTHROW : ch(c) {}
    Q_DECL_CONSTEXPR inline char toLatin1() const Q_DECL_NOTHROW { return ch; }
    Q_DECL_CONSTEXPR inline ushort unicode() const Q_DECL_NOTHROW { return ushort(uchar(ch)); }
private:
    char ch;
};
tcanabrava added inline comments.Sep 10 2018, 2:52 PM
src/widgets/axiscontrol.h
58

so?

src/widgets/axiscontrol.h
58

Sith, tomaz is correct, I just talked with him.

! In D15281#323497, @tcanabrava wrote:

can this be configured? (the metrics or imperial?)

Yes we can set it to " mm" by default, Then expose the QDoubleSpinBox::setSufix() method. Then Client will be able to change units if the user changes the mode..

src/widgets/axiscontrol.h
58

Ok no const ref for QLatin1Char. I Will "fix" this for the rest of atcore tonight

rizzitello updated this revision to Diff 41364.Sep 10 2018, 8:22 PM
  • Don't use const ref for QLatin1Char
  • Add AxisWidget::setUnits() to allow client to set spinbox suffix
rizzitello updated this revision to Diff 41365.Sep 10 2018, 9:02 PM
  • simplify
  • Detect if the button is negitive or positive movement.
  • Adjust directions so they are correct for Z and E
  • Doxygen comments

Where will the user be able to set the units? it should have a combobox on this widget to choose those.

  • Add a selector for units
  • Connect the unit selector to atcore
rizzitello edited the summary of this revision. (Show Details)Sep 11 2018, 1:00 AM
rizzitello marked 5 inline comments as done.
laysrodrigues accepted this revision.Sep 14 2018, 7:05 PM
This revision is now accepted and ready to land.Sep 14 2018, 7:05 PM
rizzitello edited the summary of this revision. (Show Details)
  • rebase
rizzitello edited the summary of this revision. (Show Details)Sep 16 2018, 3:24 AM
tcanabrava accepted this revision.Sep 17 2018, 8:23 AM
rizzitello closed this revision.Sep 18 2018, 7:54 PM