Skip to content

bugfix for marlin #822

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.

Sign up for GitHub

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

Closed
MangaValk wants to merge 1 commit into from
Closed

bugfix for marlin #822

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion libraries/Servo/src/Servo.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,8 +110,9 @@ class Servo {
int read(); // returns current pulse width as an angle between 0 and 180 degrees
int readMicroseconds(); // returns current pulse width in microseconds for this servo (was read_us() in first release)
bool attached(); // return true if this servo is attached, otherwise false
private:
protected:
Copy link
Contributor

@ABOSTM ABOSTM Dec 9, 2019

Choose a reason for hiding this comment

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

I don't see a reason to change this attribute to protected instead of private.
This variable should not be modified outside of the class.
I can understand that you need to read this variable, but in this case we can rather implement an accessor.

Copy link
Author

@MangaValk MangaValk Dec 9, 2019

Choose a reason for hiding this comment

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

A getter function would be fine too. I'm not a professional c++ programmer, so I thought a protected var would be fine.

uint8_t servoIndex; // index into the channel data for this servo
private:
int8_t min; // minimum is this value times 4 added to MIN_PULSE_WIDTH
int8_t max; // maximum is this value times 4 added to MAX_PULSE_WIDTH
};
Expand Down
2 changes: 1 addition & 1 deletion libraries/Servo/src/stm32/Servo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ static volatile int8_t timerChannel[_Nbr_16timers] = {-1}; // counter for the se

static HardwareTimer TimerServo(TIMER_SERVO);

uint8_t ServoCount = 0; // the total number of attached servos
static uint8_t ServoCount = 0; // the total number of attached servos
Copy link
Contributor

@ABOSTM ABOSTM Dec 9, 2019

Choose a reason for hiding this comment

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

From my point of view there is no restriction to define this variable static.
Nevertheless, if this cause you issue in Marlin, this probably means you have a variable with the same name ? Can you confirm ? maybe you also need to take some action on marlin to avoid conflict too?

Copy link
Author

@MangaValk MangaValk Dec 9, 2019

Choose a reason for hiding this comment

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

Here is link to the marlin change I made MarlinFirmware/Marlin#16151 (review)

Somehow this made it work, if I turn it back to normal it stops working.


#define SERVO_MIN() (MIN_PULSE_WIDTH - this->min * 4) // minimum value in uS for this servo
#define SERVO_MAX() (MAX_PULSE_WIDTH - this->max * 4) // maximum value in uS for this servo
Expand Down