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

Fix Not by reference. But the value was updated. #3279

Open
wants to merge 2 commits into
base: master
from

Conversation

@tanakamasayuki
Copy link
Contributor

commented Sep 26, 2019

No description provided.

retrieveCharacteristics();
}
log_v("<< getCharacteristics() for service: %s", getUUID().toString().c_str());
*pCharacteristicMap = m_characteristicMapByHandle;

This comment has been minimized.

Copy link
@me-no-dev

me-no-dev Sep 26, 2019

Member

why not by reference?

@tanakamasayuki

This comment has been minimized.

Copy link
Contributor Author

commented Sep 26, 2019

Now 1.0.3

void BLERemoteService::getCharacteristics(std::map<uint16_t, BLERemoteCharacteristic*>* pCharacteristicMap) {
	pCharacteristicMap = &m_characteristicMapByHandle;
}

Use case

std::map<uint16_t, BLERemoteCharacteristic*> *mapCharacteristics;
pRemoteService->getCharacteristics(mapCharacteristics);

mapCharacteristics is not update.
Because, pCharacteristicMap is copy value.(not reference)

Change to reference

void BLERemoteService::getCharacteristics(std::map<uint16_t, BLERemoteCharacteristic*>* &pCharacteristicMap) {
	if (!m_haveCharacteristics) {
		retrieveCharacteristics();
	}
	pCharacteristicMap = &m_characteristicMapByHandle;
}

Argument type : pCharacteristicMap -> &pCharacteristicMap
My recommendation.
But, The declaration changes.


I want m_characteristicMapByHandle.
Some BLE HID devices have multiple types of notifications with the same UUID.
 

@me-no-dev

This comment has been minimized.

Copy link
Member

commented Sep 29, 2019

pCharacteristicMap is a pointer to where m_characteristicMapByHandle is stored. This is what the & there means. Get the address of the object. Maybe the issue is elsewhere?

@tanakamasayuki

This comment has been minimized.

Copy link
Contributor Author

commented Oct 3, 2019

commnet

Now the value is copied and another variable is passed.
So even if you update, the original variable will not change.

Some countermeasure is required.

  • to reference
  • fix pointer
  • remove function

This function is difficult.
My recommendation is reference or remove.
But, Fix with less impact.

test code

int  a = 1;
int* b = NULL;
int  c = 0;

// now function
void func_copy( int *d ) {
  // d is copy value.(&d != &b)
  Serial.printf("func_copy\n");
  Serial.printf(" d:%p\n", d);
  Serial.printf(" &d:%p\n", &d);
  d = &a;
}

// use reference
void func_reference( int* &d ) {
  // d is original value.(&d == &b)
  Serial.printf("func_reference\n");
  Serial.printf(" d:%p\n", d);
  Serial.printf(" &d:%p\n", &d);
  d = &a;
}

// use pointer
void func_pointer( int *d ) {
  // d is original pointer address.(d == &b)
  Serial.printf("func_pointer\n");
  Serial.printf(" d:%p\n", d);
  Serial.printf(" &d:%p\n", &d);
  *d = a;
}

void setup() {
  Serial.begin(115200);
  delay(1000);

  // Init
  Serial.printf("Init\n");
  Serial.printf(" &a:%p\n", &a);
  Serial.printf(" &b:%p\n", &b);
  Serial.printf(" &c:%p\n", &c);
  Serial.printf("=================\n");

  // Now(copy value)
  b = NULL;
  func_copy(b);
  Serial.printf(" finish b:%p\n", b);

  Serial.printf("=================\n");

  // to reference
  b = NULL;
  func_reference(b);
  Serial.printf(" finish b:%p\n", b);

  Serial.printf("=================\n");

  // to pointer
  func_pointer(&c);
  Serial.printf(" finish c:%d\n", c);
}

void loop() {
}

output

Init
 &a:0x3ffbebe8
 &b:0x3ffc00c0
 &c:0x3ffc00bc
=================
func_copy
 d:0x0
 &d:0x3ffb1f5c
 finish b:0x0
=================
func_reference
 d:0x0
 &d:0x3ffc00c0
 finish b:0x3ffbebe8
=================
func_pointer
 d:0x3ffc00bc
 &d:0x3ffb1f5c
 finish c:1

And, warning output

C:\Users\tanaka\AppData\Local\Arduino15\packages\esp32\hardware\esp32\1.0.4\libraries\BLE\src\BLERemoteService.cpp: In member function 'void BLERemoteService::getCharacteristics(std::map<short unsigned int, BLERemoteCharacteristic*>*)':

C:\Users\tanaka\AppData\Local\Arduino15\packages\esp32\hardware\esp32\1.0.4\libraries\BLE\src\BLERemoteService.cpp:246:89: warning: parameter 'pCharacteristicMap' set but not used [-Wunused-but-set-parameter]

 void BLERemoteService::getCharacteristics(std::map<uint16_t, BLERemoteCharacteristic*>* pCharacteristicMap) {

                                                                                         ^
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.